コードレビューの心構え：Google Engineering Pracctices Documentationの要約（一部抜粋＆加筆）

本ページの目的

本ページはコードレビューに関する資料
Google Engineering Practices Documentation（日本語訳）
から執筆者が業務で役立つと思った内容を抜粋し、要約・加筆したものです。

Google Engineering Practices Documentationとは？

Googleが社内で採用しているコードレビューのガイドラインをまとめたものです。

書いていること

プログラミング言語やドメインによらない汎用的な内容に絞って書かれています。その分、心構え的な抽象度の高い内容が多めです。

書いていないこと

具体的なアクションや例。例えば、「保守性は大事」のような話題があっても、どのようなコードが保守性が高いといえるかについての、具体例や詳細な説明はありません。

執筆者の境遇

本記事はオリジナルままではなく、執筆者の視点で抜粋・加筆をしています。
コーディングに対する姿勢は開発者の立場や環境に左右されるので、境遇が近い人には参考になる部分があるかもですし、異なる人にとっては全く役に立たないかもしれません。
本記事がどのような視点でまとめているかがわかるように、執筆者の境遇を簡単に記載しておきます。

• 受託会社でデータ分析をやってます。
• 現在は社内向け集計ツールの開発・運用を担当しています。
  • かき捨てではなく、継続的に使われるコードを書きます。
  • ただし、あくまで利用者が社内なので、社外向けサービスやプロダクトの開発ほどコーディングに対する厳格さは求められていません。
• SQL、Pythonを使うことが多いです。

🧑‍💻レビュアーのためのガイド

コードレビューの目的

コードレビューの目的は、コードの品質を確認し、コードベース（リポジトリ）の健康状態を改善すること。
「健康的である」とは以下を満たしている状態（明示的な説明なかったので執筆者の理解で補足）

• 求められた機能を備えている（大前提）
• 第三者が見て理解しやすい（一貫性がある・適度にコメントがある・命名が適切など）
• 保守がしやすい（ハードコーディングしていない・疎結合になっているなど）
  「動きさえすればOK」の精神*で書いたコードをどんどん追加していくと、健康状態は悪化する
  *このスタンスを完全否定するつもりはないです。状況に応じた使い分けが大事です。

コードレビューの心得

本質的でない部分に時間をかけすぎない

• 書かれたコードが健康状態を改善すると判断できるならば、レビュアーはプルリクエストを積極的に承認すべき。完璧に正しいコードなど存在しないのに、それを目指して重要でない些末な修正を求めて時間をかけさせてはいけない。
• 修正してほしいと感じた点があれば、それが単なる自分の好みによるものか、本当にリポジトリの健康状態を改善するためのものなのかを考える。その修正が必要である理由を言語化して説明できないのであれば、それは好みの問題として最終的にコード作成側に合わせる。

問題の指摘だけでなく、アドバイスや良い点のフィードバックも忘れずに！

• コードレビューはコード作成者が新たなことを学ぶチャンスでもある。そのため、改善できそうな点はコメントを残す。知識の共有は長い目で見てリポジトリの健康状態を維持・改善することに役立つ。
  • ただし、上で記載したようにあまり些末なことで時間をかけさせてはいけないので、伝え方には注意する。例えば、重要でないアドバイスや指摘であれば、「Nit:」（細かい指摘の意）や「comment:」のような前置きをつけて、対応が必須ではない旨を添えるなど。
  • 参考：コードレビューにラベルを付けるだけでチームの心理的安全性を高めた話
• コードレビューでは間違いばかりに目が行きがちだが、実装の中に良い点を見つけた場合は積極的に伝える！
  • 開発者が良い点を再認識できる、無意識に実装していて部分が実は良かったと思わぬ気付きになる、などのメリットがある。もちろん開発者のモチベーション維持にも効果あり（レビューではどうしても指摘が多くなりがちなので）。

レビューは1行ずつ丁寧に

※「変更箇所を見る」に書いているように、目を通す順番は工夫すべき
コードが読みづらく、そのせいでレビューが遅れているならば、作成者にコードを見やすく整理してもらうよう依頼してもよい（それは開発チームの将来のためにもなる）

コードレビューの観点

観点	気を付けて確認したい点
機能性	- その変更はコード作成者の意図通りに動作するか？ - コード作成者の変更意図はユーザー*にとって適切か？ - 一般に、レビューを依頼する前にコード作成者側で動作確認を行っていると期待できるが、レビュアーはエッジケースを想定する、エンドユーザー目線で確認するなど、コードを読むだけではわからない不具合についても厳しく確認すべき
コンテキスト	-プルリクエスト確認時、コードの変更差分だけを確認することもできるが、変更箇所が影響する範囲やシステム全体のコンテキストを考慮して確認することを推奨する。 - コードだけでなく、関連するドキュメントも更新できているか（あるいは更新予定か）確認する。
複雑性	変更は複雑*になっていないか？ *複雑⁼コードを読んでも理解するのが難しく、将来このコードを使ったり、修正したりする人が不具合を生み出す可能性がある状態（例えば、やたら処理が長く役割が明確でない関数など）
命名	- あらゆるもの（変数やテーブル、ファイル名など）に適切な名前がついているか？ 適切とは、それが何であるか/何をするかを伝えるのに十分な額、読むのに困難にならない程度に短いものを指す。 - （命名に含めてよいかわからないが）ディレクトリの構成はわかりやすくなっているか？（例えば、必要なファイルが雑多に1つのディレクトリ配下にまとめられていると処理の関係や役割がわかりにくい）
コメント	- コメントは明確で有意義なものか？ - コメントでは「なぜ」（意図）を残すべきで、「何」をしているかはコードを読めばわかるはずなので説明する必要はない。また、不要なコメントが多いと、コード修正時に変更箇所が増えて無駄な工数が発生する。 何をしているかをコメントで詳しく説明する必要がある状態なら、コードをシンプル化することを考えたほうがいい。

※上記は記載の順番で確認するのが良いと思います。例えば、機能性の点で不具合があればコードの修正が発生し、その他のチェックもやり直しになるためです。

変更箇所を見る

レビューの工程として下記の順序が提唱されている。
この順序は、重大な問題があった場合になるべくそれを早く検知し、開発者にフィードバックできるようにするためのものである。

1. 変更を広く眺める

そもそも変更の目的や変更箇所を間違えていないかをざっと確認する。

2. 変更の主要部分を調べる

主要部分に問題が生じていると、枝葉部分も書き直しになる場合が多い。主要部分の問題を早めに見つけることで、余計な確認を減らす・修正時間を確保する狙いがある。

3. 変更の残り部分を実行順序を理解しながら確認する

上記1,2が問題なければ詳細をじっくり理解しながら確認する。

レビューコメント

• コメントは丁寧に。必ずコードに関してコメントして、開発者自身は批判しないように。
  （適切な例になっているか自信がないですが…）
  • ❌ 悪い例：「どうしてこんなに無駄の多い処理をさせているのか？」
  • ✅ 良い例：「コードのこの部分は〇〇という処理をさせる上で無駄が発生しています。△△を使ってより効率的に処理させることが可能です」
• コメントの意図を提示する。改善案を提示した場合は、なぜその方法がコードの健康状態をより良くするのか理由を説明する。
• 適度に指示を与える。
  • 問題点を指摘して、解決方法を開発者に任せるのは自分で考えてもらう機会になる・レビュアーの負担が減る等のメリットがある。ただし、時にはレビュアーのほうから解決方法を提案したり、コードを書いて見せることも開発者の学びになるため、バランスよく使い分けましょう。

🛠️コード作成者のためのガイド

PRの説明は適切に書きましょう

• レビュアーの負荷軽減、将来（自分含む）誰かがコードを見たときにどんな変更を加えたのかがかるという点で重要です。
• 事前に記載項目を決めてテンプレートを作っておくと楽です。
  • フォーマットが決まっていると確認する際に楽
  • フォーマットが決まっていると書く側も都度構成を考えなくていいので楽
  • 参考：Pull Requestのレビュー負荷を軽減し、開発生産性を向上するためにチームで取り組んだこと Pull Requestテンプレートの拡充

小さなPRを心がけましょう

• 単純に小さければよいという話ではない。ただ、小さいPRのほうがレビュースピード、精度は向上するので、独立した変更をいくつもまとめた無駄に大きいPRはやめたほうがいいかも、という話。
• 参考：巨大なPRだと何が良くないかについてはこちら → Pull Requestを小さくする戦略 - 開発チームのパフォーマンス向上のための第一歩 巨大なプルリクの弊害