Scrum ではコードレビューをどうやっているか？

森崎先生のソフトウェアレビューの講演を聴いて、今やっているレビューの方法をまとめときたいと思ったので、まとめてみます。今回は、コードレビューの話です。Scrum ではといっていますが、レビューのやり方はチームによって違うので、あくまでも例ですよ。PBI とか、仕様、ドメインモデルのレビューの話はまたこんど。

レビューの目的は、もちろん作成するプロダクトの品質向上です。障害を検出するのも、もちろん目的ではあるのですが、それ以降のスプリントで作成されるコードで同じ障害を作り込まないのが目的としては大きいです。そのため、レビューはプロジェクトもしくはチーム立ち上げ後、数スプリントで重点的にやります。後はスプリントの振り返りでレビューをやりたいが出てきたら、チームで決めます。

レビューのやり方

基本はチーム全員で集まってやります。最大２時間。それ以上やっても集中力が続かないので。プロジェクタで対象のコードを映しながらやります。PO も来てもらうといいですね。

対象のソースコードは、とくに実装上に不安があるところをチームメンバーの自己申告で。場合によっては、メトリクスを取得して McCabe 循環複雑度が大きいところをやったりします。

＃はじめにテストを実行して赤くならないこと確認してから始めます。

レビューは、コードを書く元になった PBI を確認してから、読み手がそのコードを説明します。とくにどういう意図でそのようなコードになったのかを説明してもらいます。

メンバーは説明を聞きながらコードを追ってコメントします。重要なのはコードから、説明された意図が読み取れるかですね。説明を聞きながらスムースにコードを追えるかどうか。コードをうまく追えないときは、なんか変なことになっていますね。あと、障害とか改善が必要であるというコメント以外に、うまく書けている、このあたりの実装が格好いいとか、ポジティブなコメントもします。

ディスカッションの上、バグであるとか、改善の必要があることがわかったら、ソースコードにコメントとして書き込みます。

バグの場合

// FIXME: CR日付　コメント

改善が必要な場合

// TODO: CR日付　コメント

レビューが終了したら、テストを実行してコードを壊してないことを確認してから、リポジトリにコミットしておきます。書きませんでしたが、次回レビューをやるときには、前回のレビューコメントがどうなっているかを確認します。

レビューで気をつけていること

森崎先生の講演でも強調されていましたけれど、レビューは書いた人を責めるためではなく、障害を後工程に残さないこと、そして設計を改善するためにやります。だから、コメントがあくまでもコードに対するもので、人に対してでないことは十分注意する必要があります。ScrumMaster の出番。

ちょっとやっていて面白いと思われるプラクティスは、初めてのバグ、障害を出した人はほめられる、というところですね。「うわ、このバグ超絶技巧！」とかね。テストで検出するのが難しいバグが早めに見つかるのは、後の仕事が楽になる良い兆候ですから。超絶バグが出てきたら、同様のバグがないか、PBI 単位でのレビューが終わってから確認します。

以前のレビューで指摘されたのと同じバグを出すと、「ダサい」という評価が待っています。３回目だと、朝会の遅刻と同じくらいのペナルティですね。おやつ基金か飲み会基金に罰金なことが多いです。３回目という評価をもらうと、だいたい次のふりかえりで、４回目をやらないような Try を設定してみます。コーディングルールにして壁に貼ったりもします。

チームでのレビューなので、レビューアとレビューイの役割は固定していません。一回のレビューセッションの中で、レビューアとレビューイが何回も入れ替わります。以前、コードを書かない立場で参加してたら、次のスプリントで実装タスクをもらったことがあります :)

まとめ

レビューを全員集まってやると、その間の実装は止まりますし、みんな疲れるので、結構コストはかかっています。ただ、よい設計感覚を共有するとか、コード全体をみんなが理解できるようになるとか、コードのスタイルがそろってくるとか、いろいろなメリットがあります。

スキルが低いので厳格なコーディングルールを適用しようとか、ネーミングルールを統一しようとかやるより、プロジェクトの早い時期にしょっちゅうレビューをやるのが有効だと思っています。

以上、ちょっとでもご参考になれば。ツッコミ、コメントをお待ちしております。