View on GitHub

google-engineering-practices

Google's Engineering Practices Documentation Japanese Translation

レビューで CL をナビゲートする

まとめ

ここまででコードレビューで期待するべきことが理解できたと思います。それでは、複数のファイルに分散したレビューを最も効率的に行うには、どのような方法で行えばよいのでしょうか？ 要点は以下のとおりです。

  1. その変更は全体として意味のあるものとなっているでしょうか？ よい CL の説明が書かれているでしょうか？
  2. 変更の最も主要な部分を最初に読んでください。CL 全体としてよく設計されていますか？
  3. CL の残りの部分を読み、論理的に適切な順序で書かれているか確認しましょう。

ステップ1: 変更に対して広い視野を持つ

まず、CL の説明をよく読み、全体として CL が何を行っているのかを理解します。この変更は本当に意味のあるものでしょうか？ そもそもこの変更が行われるべきものでないのなら、すみやかにその変更が行われるべきではない理由を説明する返事を書いてください。こうした変更のリジェクトを行う場合には、開発者が代わりに行うべきことも提案するのがよい考えです。

たとえば、次のように返事を書きます。「このコンポーネントに対するあなたの仕事は素晴らしいと思います。ありがとう！ ですが、あなたがこの CL で変更を行っている FooWidget システムは削除する予定になっているため、今は新しい変更を加えたくないと考えています。代わりに新しい BarWidget クラスに対してリファクタリングを行うのはいかがでしょうか？」

レビュアは現在の CL をリジェクトして別の代替案を提案していますが、それを丁寧に行っていることに注意してください。このような丁寧さはとても大切なことです。たとえ相手に賛成しないときでも、私たちはお互いに開発者として尊敬の気持ちを表したいと考えているからです。

望まない変更を行う CL が少数ではなく、もっと頻繁に送られて来てしまう場合、CL を書き換える前にもっとコミュニケーションが密に行えるように、あなたのチームの開発プロセスや、外部のコントリビュータの投稿のプロセスについて再編成することを検討しなければなりません。無駄になってしまったり大幅な書き直しが必要になる大量の作業を行わせてしまうより前に、「No」と伝えた方ががよいからです。

ステップ2: CL のメインとなる部分を確認する

この CL の「メイン」となる部分を構成するファイル (複数の場合もあります) を探しましょう。多くの場合、ロジックの変更数が最も多い1つのファイルがメインとなり、CL の大部分の変更が含まれます。このメイン部分が、CL のすべての小さな変更にコンテキストを与える助けとなります。それにより、コードレビューが素早く行えるようになります。もし CL が大きすぎて、どの部分がメイン部分なのかが理解できない場合には、開発者に最初にどの部分を見るべきかを尋ねるか、CL を複数の部分に分割するようにお願いしましょう。

もしあなたが CL のメイン部分に設計の問題があると考えた場合には、たとえ残りの部分をすぐにレビューする時間がなかったとしても、すみやかにコメントを送らなければなりません。実際、設計の問題が十分明らかな場合には、レビュー対象の他のコードの多くが無くなり、問題とはならなくなるため、CL の残りの部分のレビューは時間の無駄となる可能性もあります。

こうした主要な設計に関するコメントをすみやかに送るのが重要である理由としては、以下のような主に2つの理由があります。

ステップ3: CL の残りの部分が論理的な順序になっているか最後まで見る

全体として CL に主要な設計の問題がないことが確認できたら、すべてのファイルを読み通して、変更の論理的な順序を理解するように努めましょう。同時に、レビューを忘れているファイルがないかも確認しましょう。通常、主要なファイルを確認した後であれば、コードレビューツールが表示した通りの順番にファイルを読み通すのが一番簡単でしょう。テストを最初に読むのが役に立つこともあります。それにより、開発者が変更により行おうとしている考えを理解できることがあるからです。

Next: コードレビューのスピード