コードレビューの基準
コードレビューの主な目的は、Google のコードベースにあるコードの全体的な健康状態を時間をかけて改善することです。 コードレビューのすべてのツールとプロセスは、その目的のために設計されています。
これを実現するには、さまざまなトレードオフのバランスを取る必要があります。
まず第一に、開発者は自分のタスクを進めることができなければなりません。 コードベースに改善を提出できなければ、コードベースは改善しません。 また、どんな変更に対してもレビュアーがいちいち難色を示して変更を取り入れずにいると、早晩、開発者は改善を行う意欲を失います。
一方、CL の品質を確認するのはレビュアーの義務です。CL を取り入れてコードベースのコードの全体的な健康状態 (code health) がだんだんと悪化するのは問題ですから、きちんと確認しなければなりません。 これは骨の折れるやっかいな仕事になることがあります。 というのも、多くの場合、コードの健康状態を悪化させる要因が積み重なり、コードベースの品質は徐々に下がります。 特に、チームが無視できない時間的制約に縛られていて、ゴールを達成するためにはショートカットをするしかないと感じているときには、コードベースの品質は下がりやすいものです。
また、レビュアーにはレビュー中のコードに対して所有権と責任があります。 レビュアーはコードベースの一貫性と保守可能性を維持し、「コードレビューの観点」で言及されている事項を守りたいと考えています。
こうして、私達はコードレビューに期待する基準として、次のルールを見出しました。
一般に、CL が完璧でなくても、その変更がシステムのコードの全体的な健康状態を改善すると確実にわかれば、レビュアーは CL を積極的に承認すべきである。
これはコードレビューの全ガイドラインで最上位の原則です。
もちろん、制限はあります。たとえば、CL がレビュアーが望んでいない機能をシステムに追加している場合、うまくコードが設計されてるとしてもレビュアーは承認を拒否することができます。
ここでのポイントは、「完璧な」コードといったものは存在しないということです—存在するのはより良いコードだけです。 レビュアーは CL の承認を保留したままコードのすみずみまで磨きをかけることを要求すべきではありません。 むしろ、前に進めるべきかどうかの必要性を、CL 作成者が提案している変更の重要性と比較して、バランスよく検討すべきです。 レビュアーが追求すべきは、完璧さではなく継続的な改善です。 CL が全体としてシステムの保守性、可読性、理解可能性を向上させるのなら、コードが「完璧」でないからといって数日も数週間も遅らせるべきではありません。
レビュアーは何か改善できそうな点を見つけたら、コメントを残すのに躊躇する必要はありません。 いつでも気軽にコメントを残すべきです。 その際に、もし重要でない指摘であれば、「Nit: 」(あら探しや細かい指摘という意味)のようなプレフィックスを付けて、 これはただ完全を期すための指摘なので無視してもらっても構わない、と CL 作成者に知らせると良いでしょう。
(注)このドキュメントは、システムのコードの全体的な健康状態を悪化させるとわかりきっている CL を正当化しません。 唯一それが許されるケースは、緊急事態にあります。
メンタリング
コードレビューは、開発者に言語、フレームワーク、あるいはソフトウェア設計の一般的な原則に関して新しいことを教える重要な機会になります。 開発者が新しいことを学ぶヒントになるコメントは、いつでも歓迎されます。 知識の共有は、時間をかけてシステムのコードの健康状態を改善する試みの一部分です。 一点だけ頭に入れていただきたいのは、コメントが純粋に教育目的であって、このドキュメントで説明している基準を満たす上で重要でなければ、「Nit: 」などのプレフィックスを付けるか、この CL で作成者が解決する義務はないと付け加えてください。
原則
-
技術的な事実とデータが個人的な意見と好みをくつがえす。
-
スタイルに関しては、スタイルガイドが絶対的な権威である。スタイルガイドの記載のない純粋なスタイルの選択(空白をどうするかなど)は個人の好みの問題である。スタイルはシステム内で一貫している必要がある。スタイルの選択に前例がなければ、CL 作成者のやり方を受けれるべきである。
-
ソフトウェア設計に関する論点はほとんどの場合、純粋なスタイルの問題でも個人的な好みの問題でもない。それは基本的な原則に基づいており、ただの個人的な意見によるのではなく、その原則に重点を置くべきである。有効な選択肢がいくつかある場合がある。複数の方法が同等に有効であると作成者が(データを示したり堅固な工学原理に基づいて説明したりして)証明できるとき、レビュアーは作成者の選好を受け入れるべきである。そうでない場合、選択はソフトウェア設計の標準的な原則によって決定される。
-
他のルールが適用されない場合、システムのコードの全体的な健康状態を悪化させない限りは、レビュアーは CL 作成者に現在のコードベースとの一貫性を維持するよう求めることができる。
意見の対立の解消
コードレビューで意見の対立があれば、最初のステップで必ず行うべきは、このドキュメントとCL 作成者のガイド、またこのレビュアーガイドの他のドキュメントに基づいて、開発者とレビュアーの間でコンセンサスが得られるように調整することです。
コンセンサスを得るのが特に難しいときには、レビュアーと作成者で対面でのミーティングやテレビ会議をもつほうが、コードレビューコメントのやり取りだけで対立を解消しようとするよりも効果がある場合があります。 (対面でミーティングをするとしても、未来の読者のために CL のコメントにディスカッションの結果を記録するのを忘れないでください。)
それでも状況が変わらないなら、通例では、巻き込む人を増やすのが解決する方法です。 よくあるのがより広範にチーム内で議論することです。TL に検討してもらったり、コードのメンテナーに決定してもらうようお願いしたり、技術マネージャーに助力を求めたりできます。 作成者とレビュアーが合意に達することができないからといって、CL をそのまま放置しないでください。
次: コードレビューの観点