レビューで CL を閲覧する
要約
前節でコードレビューの観点を学びましたが、では複数ファイルにまたがった変更をレビューする最も効果的な方法は何でしょうか？
- 変更はうまく機能するでしょうか？適切なディスクリプションはあるでしょうか？
- いちばん重要な変更を最初に確認してください。それは全体としてうまく設計されているでしょうか？
- CL の残りの部分を適切な順序で見てください。
ステップ 1: 変更を広く眺める
CL のディスクリプションと CL が大まかに何をしているかを確認してください。 この変更は機能しているでしょうか？ そもそもこの変更が行ってはならないものであれば、変更すべきでない理由を添えてすぐに返信してください。 そのように変更を却下する場合、代わりに何をすべきかを開発者に提案するのも良い考えです。
たとえば、次のように伝えることができます。「これに関して良い仕事をしてくださっているように思えます。ありがとう！ですが、実はあなたがここで修正した FooWidget システムは削除する方向で進めているため、今のところ新しい修正をしたくないのです。代わりに新しい BarWidget クラスをリファクタリングしていただくのはどうでしょうか？」
注意していただきたいのですが、レビュアーは現在の CL を却下して代わりの提案をしただけではなくて、それを礼儀正しく伝えました。 このような礼儀正しさは重要です。私達は意見が一致しないときでも開発者として互いを尊重し合うということを示したいからです。
変更してほしくない部分を変更する CL がいくつも送られる場合、 チームの開発プロセスや外部のコントリビューター向けに投稿されたプロセスを再検討してください。 CL が書かれる前にもっとコミュニケーションが必要です。 人々が 1 トンもの仕事をしてその後で破棄や大部分の書き直しを迫られるよりは、事前に「ノー」と言えるほうが良いでしょう。
ステップ 2: CL の主要部分を調べる
この CL の「メイン」の部分になっているファイル（1 ファイルとは限りません）を見つけてください。 よくあるのは、ロジック上の変更が最も多いファイルが一つあり、それが CL の主要部分となる場合です。 これが CL の別の小さな部分にコンテキストを提供していて、それによってコードレビューが概してスムーズになります。 CL があまりに巨大でどの部分が主要部分なのか判別できなければ、開発者にどこを最初に見るべきか質問してもよいですし、あるいは CL を複数の CL に分割するように依頼してください。
CL の主要部分に設計上の重大な問題が見つかれば、すぐにコメントを残してください。 CL の残りの部分をレビューする時間があってもコメントを送るのが先です。 実際、CL の残りの部分をレビューしても時間の無駄になるかもしれません。設計上の問題が重大なものであれば、レビュー対象の他のコードは消されることになり、見ても見なくても関係なくなるからです。
設計上の重大な問題に関するコメントをただちに送ることが大切な理由は二つあります。
- 開発者は CL を投稿すると、レビューを待ちながらその CL をベースに新しい作業をすぐに始めることがよくあります。 レビュー中の CL に設計上の重大な問題があると、開発者は次の CL もやり直さなければならなくなります。 開発者が問題含みの設計の上にさらに仕事を積み上げてしまう前に引き止めたいものです。
- 大きな設計の変更は小さな変更よりも時間がかかります。 開発者にはたいてい納期がありますが、納期を守りつつコードベースのコードの品質を保つには、CL のやり直しが重大なものであるほど、できるだけ早くやり直しに着手する必要があります。
ステップ 3: CL の残りを適切な順序で見る
全体として CL に設計上の重大な問題がないことが確認できたら、次は全ファイルを一通り見て、論理的な順序を理解するようにしてください。またその際に、レビューもれのファイルがないように気をつけてください。 主要なファイルを確認し終えたら、普通はコードレビューツールが表示してくれる順序で各ファイルを調べるのがいちばん簡単です。 主要なコードを読む前にテストをまず読むのが効果的な場合もあります。 そうするとこの変更が何をしようとしているのかイメージがつくからです。
次: コードレビューのスピード