View on GitHub

google-eng-practices-ja

Google's Engineering Practices documentation 日本語訳

コードレビューの基準

コードレビューの主な目的は、Google のコードベースにあるコードの全体的な健康状態を時間をかけて改善することです。 コードレビューのすべてのツールとプロセスは、その目的のために設計されています。

これを実現するには、さまざまなトレードオフのバランスを取る必要があります。

まず第一に、開発者は自分のタスクを進めることができなければなりません。 コードベースに改善を提出できなければ、コードベースは改善しません。 また、どんな変更に対してもレビュアーがいちいち難色を示して変更を取り入れずにいると、早晩、開発者は改善を行う意欲を失います。

一方、CL の品質を確認するのはレビュアーの義務です。CL を取り入れてコードベースのコードの全体的な健康状態 (code health) がだんだんと悪化するのは問題ですから、きちんと確認しなければなりません。 これは骨の折れるやっかいな仕事になることがあります。 というのも、多くの場合、コードの健康状態を悪化させる要因が積み重なり、コードベースの品質は徐々に下がります。 特に、チームが無視できない時間的制約に縛られていて、ゴールを達成するためにはショートカットをするしかないと感じているときには、コードベースの品質は下がりやすいものです。

また、レビュアーにはレビュー中のコードに対して所有権と責任があります。 レビュアーはコードベースの一貫性と保守可能性を維持し、「コードレビューの観点」で言及されている事項を守りたいと考えています。

こうして、私達はコードレビューに期待する基準として、次のルールを見出しました。

一般に、CL が完璧でなくても、その変更がシステムのコードの全体的な健康状態を改善すると確実にわかれば、レビュアーは CL を積極的に承認すべきである。

これはコードレビューの全ガイドラインで最上位の原則です。

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

ここでのポイントは、「完璧な」コードといったものは存在しないということです—存在するのはより良いコードだけです。 レビュアーは CL の承認を保留したままコードのすみずみまで磨きをかけることを要求すべきではありません。 むしろ、前に進めるべきかどうかの必要性を、CL 作成者が提案している変更の重要性と比較して、バランスよく検討すべきです。 レビュアーが追求すべきは、完璧さではなく継続的な改善です。 CL が全体としてシステムの保守性、可読性、理解可能性を向上させるのなら、コードが「完璧」でないからといって数日も数週間も遅らせるべきではありません。

レビュアーは何か改善できそうな点を見つけたら、コメントを残すのに躊躇する必要はありません。 いつでも気軽にコメントを残すべきです。 その際に、もし重要でない指摘であれば、「Nit: 」(あら探しや細かい指摘という意味)のようなプレフィックスを付けて、 これはただ完全を期すための指摘なので無視してもらっても構わない、と CL 作成者に知らせると良いでしょう。

(注)このドキュメントは、システムのコードの全体的な健康状態を悪化させるとわかりきっている CL を正当化しません。 唯一それが許されるケースは、緊急事態にあります。

メンタリング

コードレビューは、開発者に言語、フレームワーク、あるいはソフトウェア設計の一般的な原則に関して新しいことを教える重要な機会になります。 開発者が新しいことを学ぶヒントになるコメントは、いつでも歓迎されます。 知識の共有は、時間をかけてシステムのコードの健康状態を改善する試みの一部分です。 一点だけ頭に入れていただきたいのは、コメントが純粋に教育目的であって、このドキュメントで説明している基準を満たす上で重要でなければ、「Nit: 」などのプレフィックスを付けるか、この CL で作成者が解決する義務はないと付け加えてください。

原則

意見の対立の解消

コードレビューで意見の対立があれば、最初のステップで必ず行うべきは、このドキュメントとCL 作成者のガイド、またこのレビュアーガイドの他のドキュメントに基づいて、開発者とレビュアーの間でコンセンサスが得られるように調整することです。

コンセンサスを得るのが特に難しいときには、レビュアーと作成者で対面でのミーティングやテレビ会議をもつほうが、コードレビューコメントのやり取りだけで対立を解消しようとするよりも効果がある場合があります。 (対面でミーティングをするとしても、未来の読者のために CL のコメントにディスカッションの結果を記録するのを忘れないでください。)

それでも状況が変わらないなら、通例では、巻き込む人を増やすのが解決する方法です。 よくあるのがより広範にチーム内で議論することです。TL に検討してもらったり、コードのメンテナーに決定してもらうようお願いしたり、技術マネージャーに助力を求めたりできます。 作成者とレビュアーが合意に達することができないからといって、CL をそのまま放置しないでください。

次: コードレビューの観点