コードレビューの基準

コードレビューの主な目的は、Google のコードベースのコード全体の健全性が、時間の経過とともに改善されることを保証することです。コードレビューのツールとプロセスのすべては、この目的のために設計されています。

この目的を達成するには、一連のトレードオフの間でバランスを取る必要があります。

まずはじめに、開発者は自分たちのタスクを前に進めることができなければなりません。もしあなたがコードベースに対する改善を全く提出しなければ、コードベースが改善することは決してありません。また、レビュアがあらゆる変更を加えるのを非常に難しくしてしまえば、開発者が将来改善を加えようとするインセンティブが失われてしまいます。

一方で、各 CL が、時間が経過してもコードベースのコード全体の健全性が低下しないような高い品質を備えていることを保証することも、レビュアの義務です。しかし、それはやっかいなことに思われるかもしれません。通常コードベースでは、特にチームが厳しい時間の制約にさらされていて、目標を達成するためにショートカットを使わなければならないと感じているような場合、時間の経過とともに小さな健全性の低下が重なり、コード全体の健全性も少しずつ低下してゆくものだからです。

さらに、レビュアは、自分がレビューしたコード全体に対する所有権と責任を持ちます。レビュアは、コードベースが一貫性を持ち続け、メンテナンス性が維持され、「コードレビューで何を期待するべきか」で言及されている他のすべての事柄についても保証しようとします。

したがって、私たちがコードレビュー中に期待する基準として、次のルールを定めます。

一般に、ある CL が作業対象のシステムのコード全体の健全性を具体的に向上させる状態に一度でも達したならば、たとえその CL が完璧なものでなくても、レビュアは承認を賛成しなければならない。

これは、すべてのコードレビューガイドラインの中で最も重要な原則です。

この基準にはもちろん制限があります。たとえば、レビュアがシステムに望まない機能を追加する CL であれば、たとえコードがよく設計されていたとしても、レビュアは承認を拒否することができます。

ここでのキーポイントは、「完璧」なコードというものはどこにも存在せず、あるのはよりよいコードだけだということです。レビュアは、CL の作者に対して、承認前にコードのあらゆる細かい部分まで洗練させることを要求するべきではありません。むしろレビュアは、システムを前に進める必要性と作者が提出した変更の重要性とを比べて、バランスを取らなければなりません。レビュアが求めるべきなのは、完璧さではなく、継続的な改善です。CL が全体として、システムのメンテナンス性、リーダビリティ、理解のしやすさを改善するものであるならば、たとえそれが「完璧」なものではなくても、数日や数週間も提出を遅らせてはなりません。

レビュアには常に、もっと改善できるはずだと思う点についてコメントする自由がなければなりません。しかしそれほど重要な点ではないのなら、コメントの最初に “Nit: “ (訳注: 些細な点という意味、ニッチの単数形) のようなキーワードを付けて、作者に改善可能な点だが改善するか否かは作者に委ねる、ということを知らせてあげましょう。

注意: このドキュメントは、システム全体のコードの健全性を具体的に悪化させるような CL をチェックインすることを正当化するものでは決してありません。そのような CL は、緊急事態の時以外には許されません。

メンタリング

コードレビューには、開発者にプログラミング言語やフレームワーク、ソフトウェア設計の一般的な原則について何か新しいことを教える、という重要な役目もあります。開発者が何か新しいことを学ぶのを助けるコメントを残すのは、常に良いことです。知識を共有することは、時間の経過とともにシステムのコードの健全性を改善することの一部でもあります。ただし、もしあなたのコメントが純粋に教育的なものであり、このドキュメントで説明している基準を満たすために必須なものではない場合には、そのコメントの最初に “Nit: “ と書くか、作者には現在の CL の中で解決する義務があるわけではないと伝えることを忘れないようにしてください。

原則

• 技術的な事実やデータは、意見や個人的な好みに優先されます。

• スタイルの問題については、スタイルガイドが絶対的な権威です。純粋なスタイルに関する事柄 (ホワイトスペースなど) がスタイルガイドの中に定義されていない場合には、個人の好みの問題です。ただし、スタイルがどのようなものであっても、一貫性がなければなりません。もし過去にスタイルが存在しなければ、CL の作者のスタイルを受け入れてください。

• ソフトウェア設計のほとんどすべての側面は、純粋なスタイルの問題や個人的な好みであることは決してありません。それらは根底にある原則に基づくものであり、単に個人の意見によるものではなく、根底にある原則を重視しなければなりません。ときには有効な選択肢がいくつか存在することもあります。もし CL の作者が (データを通してであれ、強固なエンジニアリングの原則に基づいてであれ) 複数のアプローチが同等に有効であることを提示することができるのであれば、レビュアは作者の好みを受け入れるべきです。そうでなれば、選択はソフトウェア設計の標準的な原則によって決定されます。

• もし他のどのルールも適用できない場合は、システムのコード全体の健全性を悪化させない限り、レビュアは CL の作者に対して、現在のコードベースの中にあるものの一貫性を保つように求めることができます。

対立の解消

コードレビュー上で対立が生まれた場合に常に最初にしなければならないのは、CL の作者のためのガイドとこのレビュアのためのガイド中のドキュメントの内容に基づいて、開発者とレビュアの双方がコンセンサスを得ようと努めることです。

コンセンサスを得るのが特に難しくなった場合には、コードレビューのコメントだけで対立を解消しようとするよりも、レビュアと作者の間で対面のミーティングやビデオチャットをすると助けとなることがあります。(ただし、もしこれを行った場合、将来のコードベースの読者のために、CL のコメント中に議論の結果を忘れずに記録するようにしてください。)

もしそれでも対立が解消しない場合、最も一般的な方法は問題の解決を上位レベルに移譲することです。よくある移譲の経路としては、より広いチームディスカッションを行う、TL に仲裁に入ってもらう、コードのメンテナに決定を求める、Eng Manager に助けを求める、といった選択肢があります。CL の作者とレビュアが合意を得られないという理由で、CL を放置してはなりません。

Next: コードレビューで何を期待するべきか