よいレビューを行うために必要なこと

TL;DR

• レビュアー、レビューイ共に準備をして臨むことが肝要。
• レビューされているのは成果物であり、人格ではない。
• 互いにリスペクトをもって臨む。

失敗プロジェクトはレビューに何かしらの問題がある

プロジェクト完了後の振り返りではレビューの問題が必ずと言ってよいほど報告されます。

• レビュアーの時間が取れなかった
• レビュー時間を短縮するために、体裁のみレビューした
• ドキュメントが整備されていなかったため、正しくレビューができなかった　等

よく聞くのは他の工程と同じく「時間がない」です。
そして直接成果物を生成しないレビューというプロセスはないがしろにされ、品質不良につながっていきます。
見積もりが十分ではなかったこともあるかと思いますが、毎回見積もりが正しくないとするのは問題から目を背ける行為です。
どう改善できるのか見ていきましょう。

レビュー前の準備不足

時間が足りなくなる理由のひとつにレビューイ、レビュアー双方の準備不足があります。
それぞれのどんな準備が必要でしょうか。

レビューイの準備

基本的な体裁を整える

レビュー対象がコードであっても、ドキュメントであっても、事前の約束を遵守することが必要です。

たとえば対象がコードであれば、

• インデントなどのコードフォーマット
• 命名規則
• Javadocなどを含むコードコメント
• CheckstyleやSpotBugsに代表される静的解析ツールの実行と対応
• 前提となる仕様書などのドキュメント（最新化されていること！）

事前にコードが整理されていれば、レビュアーは本来確認すべき内容に集中できます。
仕様書のレビューであれば、前提となる要件をレビュアーが確認できるように提示できる必要がありますし、事前合意した体裁に合わせる必要があります。
「こんなことは当たり前」と思うかもしれませんが、実際現場に入ると疎かになっていることが多いです。
フォーマットやCheckstyleなどは事前の設定で保存時などに自動実行させることも可能です。
できることは自動化して、手間を省くことも必要です。

目的の共有

レビュー依頼を行う際に、その変更の目的を共有することが必要です。

• なぜこの変更をしたのかを明らかにする
  たとえば、WBSのタスク番号、バグ票の管理番号など、容易に紐づけができる情報を記載します。
• どのような変更したのか、どの機能が対象かを明らかにする
  前述したとおり、変更の元となる仕様書や要件を明示する必要があります。
  また、その要件に対してどのようにどの対象（ソースや仕様書）に変更を行ったのかを短く簡潔に説明します。
• やらないこと・残課題などを明らかにする
  さまざまな事情で要件にある変更の一部を保留することもあり得ます。
  そのような場合も課題管理票の番号などを明記し、レビュアーへ知らせるようにします。

小さな単位のレビューリクエスト

レビューのリクエストにたくさんの要素（複数のバグなど）を詰め込むこともレビュアーのパフォーマンスを落とす原因となります。

小さな単位のレビューリクエストについてはGoogle's Engineering Practices documentationが参考になります。

• 速くレビューできる。 レビュアーにとって、小さな CL （※）をレビューするための 5 分間を数回確保するほうが、一度の大きな CL をレビューするための 30 分をまとめて確保するよりも容易です。

• 隅々までレビューできる。 変更が大規模だとあっちこっちに詳細なコメントを大量に書かねばならず、レビュアーと作成者がストレスを感じやすくなります。ときには重要な点を見落としたり省略したりしてしまうこともあります。

• バグが混入する可能性が減る。 変更箇所が少なければ、開発者もレビュアーも CL の影響範囲が予測しやすく、バグが混入したかどうかを見分けやすくなります。

• CL が却下されても無駄になる作業が少ない。 巨大な CL を書いてからレビュアーに全体的な方向性が間違っていると言われると、多くの作業が無駄になってしまいます。

• マージしやすい。 大規模な CL での作業には時間がかかるため、マージする頃には多くのコンフリクトが発生し、頻繁にマージしなければならなくなります。

• 設計を改善しやすくなる。 小さな変更の設計やコードの健康状態を改善するほうが、大きな変更の詳細をすべて見直して設計を洗練させるよりもずっと簡単です。

• レビューによって作業がブロックされなくなる。 あなたがしようとしている変更全体のうち一部を自己完結的な CL にして提出すれば、現在の CL のレビューを待つ間にコーディング作業を続けることができます。

• ロールバックしやすい。 大きな CL は多くのファイルにわたって変更するため、最初に CL を提出してからロールバックの CL までにそれらのファイルが変更され、ロールバックが複雑になります。

※ CL：Change List（変更要求）

引用元：FujiHaruka/google-eng-practices-ja

レビュアーの時間を確保する

弊社のプロジェクトの場合、レビュアーを専門で配置している場合はほぼありません。
プロジェクト進捗のスケジュール遅延理由でよく聞かれるのは「レビューが遅れている」というものです。
レビュアー側に確認をしてみると、突然レビューを依頼されていることが多々あります。
自分の成果物の進捗に合わせて、レビュアーの時間確保も同時に進めることが肝心です。

レビュアーの準備

事前にレビュー観点を用意する

レビューへ臨む前にどのような観点でレビューを行うか、基準を用意しておくことが重要です。
基準を準備しておくことで、ブレのないレビューを実施でき、時間の短縮にもなります。
コードレビュー観点についてもGoogle's Engineering Practices documentationが参考になります。

要約を転載します。

• コードがうまく設計されている
• 機能性がコードのユーザーにとって適切である
• UI の変更がある場合、よく考えられていて見た目も適切である
• 並行処理がある場合、安全に行われている
• コードが必要以上に複雑でない
• 開発者は将来必要になるかもしれないものではなく、現在必要だとわかっているものを実装している
• コードには適切なユニットテストがある
• テストがうまく設計されている
• 開発者はあらゆるものに明確な名前を使った
• コメントは明確で有意義なもので、「何」ではなく「なぜ」を説明している
• コードは適切にドキュメント化されている
• コードはスタイルガイドに準拠している

引用元：FujiHaruka/google-eng-practices-ja

また、次のような観点も重要です。

• セキュリティ上の欠陥がないこと
• パフォーマンス上問題がないこと

時間を確保する

理由はレビューイと同じです。
レビューを担当するとなったら、対象の成果物の進捗状況に目を向けている必要があります。

レビューに臨む姿勢

レビューに取り組む姿勢を双方に一致させることで、レビューの効果をあげられますし、時間の節約にもなります。

成果物はあなたのものではない

レビューイが提出する成果物はユーザーに提供するものであり、チーム内で共有されるものです。
その認識の下、作成されていることが前提となります。
たとえば、個人的なこだわりや最新の技術を利用したトリッキーなコード（それがいかにすばらしいとしても！！）は含めるべきでありません。
「かかわるすべての人が理解できるもの」となっているか、客観的に判断する必要があります。
レビュアーについても同様のことがいえます。
コードや仕様の表現のアプローチはひとつとは限りません。
前述したように「かかわるすべての人が理解できるもの」から逸脱していないのであれば、レビューイのアプローチは尊重される必要があります。
しかしながら、性能を高めることができる、より可読性を上げることができるなど、ポジティブな代案があれば積極的に行うべきです。
また、レビュアーは「理解できる」フィードバックコメントができているかも同様に大切なことです。

指摘された箇所だけにフォーカスしない（横展開）

レビューイは指摘された箇所だけを対応するのではなく、指摘の根本原因を考察し、他に同様の箇所がないか横展開を実施します。

レビュー対象は成果物である

誰しもレビューの場で人格を攻撃された（またはそう感じた）経験があるのではないでしょうか。
特殊な例を除いて「こいつを攻撃してやろう」と思ってレビューに臨むレビュアーはいません。
レビュアーが指摘しているのは成果物の内容であり、あなた自身ではないというマインドで臨みましょう。
レビューコメントに対して怒りが湧いてきても、6秒数えて、「この人はどうしてこんなことを言っているのか」を考えてみましょう。
そして、レビュアーが間違っていると確信が持てるのであれば、建設的な形で異議を唱えましょう。
レビュー時のコメントをすべて適用させる必要はありません。
不要である、または過剰であると感じる場合もその旨をレビュアーに説明しましょう。
きちんと説明すれば、かつその説明が成果物に対してポジティブな影響を与えるのであれば、レビュアーは理解してくれるでしょう。
レビュアーも自分の指摘が人格攻撃になっていないかに注意し続けることが必要です。
成果物を改善できるポジティブなフィードバック、提案になっているかを考えましょう。
提案の意図をレビュアーレビューイ互いに理解する（理解させる）努力をすることが肝要です。

どうして人格攻撃になるのかを考える

「そんなこともできないのか」「今まで何をやってきたのか」「理解してやってる？」

どうしてこのような発言になってしまうのかも考える必要があります。
前述しましたが、特殊な例を除けば人格攻撃をしたい人はいません。
それでもこのような発言がでてしまうのは、レビュアーが成果物に真剣に向き合っているにもかかわらず、成果物などから同じ真剣さが伝わってきていないことがあるのではないかと思います。

• 事前にチームで約束したフォーマットが守られていなかった
• 前提となるドキュメントを用意していなかった（チェックリストの記入など）
• 自分のやっていることが説明できない（謎のソースをコピペした）

理由はたくさんあると思いますが、お互いにリスペクトをもって臨めば人格攻撃にはならないのではないかと思います。

まとめ

• レビューはお互いに事前の準備して臨む
• 時間の確保も準備に含まれる
• レビューの対象は成果物
• 人格攻撃をしたい人いない（例外はある）
• 互いに敬意をもって臨むことが効率的なレビューへの第一歩