Googleのエンジニアリングプラクティスドキュメント

Googleは、すべての言語とプロジェクトに適用される多くの一般的なエンジニアリングプラクティスを持っています。これらのドキュメントは、私たちが時間をかけて開発してきたさまざまなベストプラクティスの集合的な経験を表しています。オープンソースプロジェクトや他の組織も、この知識を活用できる可能性があるため、できる限り公開するように取り組んでいます。

現在、以下のドキュメントが含まれています：

• Googleのコードレビューガイドライン（実際には2つの別々のドキュメントです）：
  • コードリビューアのガイド
  • 変更作成者のガイド

用語

これらのドキュメントの一部には、Google内部の用語が使用されていますが、外部の読者のためにここで説明します：

• CL：「changelist」の略で、バージョン管理に提出されたまたはコードレビュー中の、1つの自己完結型の変更を意味します。他の組織では、これを「変更」、「パッチ」、または「プルリクエスト」と呼ぶことがよくあります。
• LGTM：「Looks Good to Me」の略で、コードリビューアがCLを承認する際に言う言葉です。

ライセンス

このプロジェクトのドキュメントは、CC-By 3.0ライセンスの下でライセンスされています。このライセンスでは、これらのドキュメントを共有することを奨励しています。詳細については、https://creativecommons.org/licenses/by/3.0/を参照してください。

はじめに

コードレビューとは、コードの著者以外の人がそのコードを検討するプロセスです。

Googleでは、コードレビューを通じてコードと製品の品質を維持しています。

このドキュメントは、Googleのコードレビューのプロセスとポリシーの正式な説明です。

このページは、コードレビューのプロセスの概要です。このガイドの一部となる他の2つの大きなドキュメントがあります。

• コードレビューの方法: コードリビューアのための詳細なガイドです。
• CL著者ガイド: レビューを受けるCLを作成する開発者のための詳細なガイドです。

コードリビューアは何を見るのか？

コードレビューでは以下の点に注意する必要があります：

• デザイン：コードはシステムに適切に設計されていますか？
• 機能性：コードは作者の意図通りに動作していますか？コードの動作はユーザーにとって良いですか？
• 複雑さ：コードはもっとシンプルにできますか？他の開発者が将来このコードに出くわしたときに簡単に理解して使用できますか？
• テスト：コードには正しい自動テストがありますか？それらは設計が良いですか？
• 命名：開発者は変数、クラス、メソッドなどに明確な名前を選びましたか？
• コメント：コメントは明確で有用ですか？
• スタイル：コードは私たちのスタイルガイドに従っていますか？
• ドキュメンテーション：開発者は関連するドキュメントも更新しましたか？

詳細については、**コードレビューの方法**を参照してください。

最適なリビューアの選択

一般的には、できるだけ早くレビューに対応できる「最適な」リビューアを見つけることが望ましいです。

最適なリビューアとは、あなたが書いているコードの部分に対して、最も詳細かつ正確なレビューをしてくれる人のことです。通常、これはコードの所有者であり、OWNERSファイルに記載されている人々かもしれません。時には、異なる人々に異なる部分のレビューを依頼することもあります。

理想的なリビューアを見つけた場合でも、その人が利用できない場合は、少なくとも変更に関して彼らをCCに追加するべきです。

直接対面でのレビュー（およびペアプログラミング）

もし、コードの一部を資格のある人とペアプログラミングで行い、そのコードに対して良いコードレビューが行われた場合、そのコードはレビュー済みとみなされます。

また、対面でのコードレビューも行うことができます。リビューアが質問をし、変更を行った開発者は問われたときにのみ話すことができます。

関連情報

• コードレビューの方法: コードリビューアのための詳細なガイド。
• CL作成者ガイド: レビューを受けるCLを作成する開発者のための詳細なガイド。

CL作成者のコードレビューを乗り越えるためのガイド

このセクションのページには、コードレビューを行う開発者のためのベストプラクティスが含まれています。これらのガイドラインは、レビューをより速く、より高品質な結果で終えるのに役立つはずです。すべてを読む必要はありませんが、これらはすべてのGoogleの開発者に適用されることを意図しており、多くの人々が全体を読むことが役立つと感じています。

• 良いCLの説明の書き方
• 小さなCL
• レビューコメントの処理方法

また、コードレビューを行う人には詳細なガイダンスを提供するコードレビューの方法も参照してください。

良いCLの説明文を書く

CLの説明文は、何が変更されているのかと、なぜその変更が行われたのかを公に記録するものです。これは、バージョン管理の履歴の一部となり、レビュワー以外の数百人の人々によって何年もの間読まれる可能性があります。

将来の開発者は、説明文に基づいてCLを検索するでしょう。将来の誰かが、関連性のある変更のかすかな記憶を持っているが、具体的な情報が手元にないために、あなたの変更を探しているかもしれません。重要な情報がコードではなく説明文にある場合、彼らがあなたのCLを見つけるのはずっと難しくなります。

最初の行

• 行われていることの短い要約。
• 命令として書かれた完全な文。
• 空行で区切る。

CLの説明の最初の行は、具体的にCLによって何が行われているかの短い要約である。その後に空行を置く。これはバージョン管理の履歴の要約に表示されるため、将来のコード検索者があなたのCLやその説明全体を読まなくても、実際にCLが何をしたのか、他のCLとどのように異なるのかを理解するのに十分な情報であるべきです。つまり、最初の行は単独で成立し、読者がコードの履歴をスキャンするのをはるかに速くすることができるようにする必要があります。

最初の行を短く、焦点を絞り、要点に留めるようにしてください。読者への明確さと有用性が最優先です。

伝統的に、CLの説明の最初の行は、命令文（命令文）として書かれた完全な文です。例えば、「FizzBuzz RPCを削除し、新しいシステムで置き換える」と言う代わりに、「FizzBuzz RPCを削除して新しいシステムで置き換える」と書きます。ただし、説明の残りを命令文として書く必要はありません。

本文は情報提供です

最初の行は短く、焦点を絞った要約であるべきです。その他の説明では、変更リストを全体的に理解するために読者が必要とする補足情報や詳細を含めるべきです。解決されている問題の簡単な説明や、なぜこれが最善のアプローチなのかを含めることがあります。アプローチの欠点がある場合は、それも述べるべきです。関連する場合、バグ番号やベンチマーク結果、設計文書へのリンクなどの背景情報も含めることができます。

外部リソースへのリンクを含める場合、アクセス制限や保持ポリシーにより将来の読者には表示されない可能性があることを考慮してください。可能な限り、レビュワーや将来の読者がCLを理解するための十分な文脈を含めてください。

小さなCLでも細部に注意を払う価値があります。CLを文脈に置いてください。

不適切なCLの説明

「バグを修正する」というCLの説明は不十分です。どのバグですか？それを修正するために何をしましたか？ 同様に不適切な説明には以下のものがあります：

• 「ビルドを修正する」
• 「パッチを追加する」
• 「コードをAからBに移動する」
• 「フェーズ1」
• 「便利な関数を追加する」
• 「奇妙なURLを削除する」

これらのいくつかは実際のCLの説明です。短いですが、十分な有用な情報を提供していません。

良いCLの説明例

以下に、良い説明の例をいくつか示します。

機能変更

例：

RPC: RPCサーバーメッセージフリーリストのサイズ制限を削除します。

FizzBuzzのようなサーバーは非常に大きなメッセージを持っており、再利用することで利益を得ることができます。 フリーリストを大きくし、フリーリストのエントリを時間をかけて解放するゴルーチンを追加します。 これにより、アイドル状態のサーバーは最終的にすべてのフリーリストエントリを解放します。

最初の数語でCLが実際に何をするのかを説明します。残りの説明では、解決される問題、これがなぜ良い解決策なのか、そして具体的な実装についてもう少し情報を話します。

リファクタリング

例：

タスクを構築し、そのTimeStrメソッドとNowメソッドを使用するために、TimeKeeperを使用します。

タスクにNowメソッドを追加し、borglet()のゲッターメソッドを削除します（これは、OOMCandidateがborgletのNowメソッドを呼び出すためにのみ使用されていました）。これにより、Borgletに委譲するメソッドが置き換えられます。

タスクがNowを提供できるようにすることは、Borgletへの依存を排除するための一歩です。最終的には、タスクからNowを取得する依存関係を直接TimeKeeperを使用するように変更する必要がありますが、これはリファクタリングを段階的に行うための配慮です。

Borgletの階層を長期的にリファクタリングする目標を続けます。

最初の行は、CLが何を行い、これが過去からの変更であることを説明しています。説明の残りの部分は、具体的な実装、CLの文脈、解決策が理想的ではないこと、および将来の方向性について説明しています。また、なぜこの変更が行われるのかも説明しています。

コンテキストが必要な小さなCL

例：

status.pyのためのPython3のビルドルールを作成します。

これにより、既にPython3を使用しているコンシューマーは、元のstatusのビルドルールの隣にあるルールに依存することができます。これは、自分自身のツリーのどこかではなく、元のビルドルールの隣に依存することを意味します。これにより、新しいコンシューマーは、可能な限りPython3を使用することが奨励され、現在作業中のいくつかの自動ビルドファイルのリファクタリングツールが大幅に簡素化されます。

最初の文は実際に何が行われているかを説明しています。残りの説明は、変更がなぜ行われているのかを説明し、レビュアーに多くの文脈を提供します。

タグの使用

タグは手動で入力されるラベルであり、CL（変更リスト）を分類するために使用できます。これらはツールによってサポートされる場合もありますし、チームの慣習として使用される場合もあります。

例：

• "[tag]"
• "[a longer tag]"
• "#tag"
• "tag:"

タグの使用は任意です。

タグを追加する際には、CLの説明の本文または最初の行に配置するかどうかを考慮してください。最初の行でのタグの使用は、内容をわかりにくくする可能性があるため、制限してください。

タグの使用例（タグありとタグなし）：

// タグは最初の行に短く保つ場合は問題ありません。
[banana] バナナを食べる前に皮をむいてください。

// タグはコンテンツ内にインラインで配置することもできます。
バナナを食べる前に #banana をむいてください。

// タグは任意です。
バナナを食べる前に皮をむいてください。

// タグが短く保たれている場合、複数のタグを使用することもできます。
#banana #apple: 果物のかごを組み立てる。

// タグはCLの説明のどこにでも配置できます。
> 果物のかごを組み立てる。
>
> #banana #apple

// タグが多すぎる（または長すぎる）と最初の行が混雑します。
//
// 代わりに、タグを説明の本文に移動したり、短縮したりできるかどうかを検討してください。
[banana peeler factory factory][apple picking service] 果物のかごを組み立てる。

生成されたCLの説明

一部のCLはツールによって生成されます。できる限り、その説明もここでのアドバイスに従うべきです。つまり、最初の行は短く、焦点を絞り、単独で成立し、CLの説明本文には、レビュワーや将来のコード検索者が各CLの効果を理解するのに役立つ情報を含めるべきです。

CLを提出する前に説明を確認してください

CLはレビュー中に大きな変更が加えられることがあります。CLを提出する前に、CLの説明を再確認して、説明がCLの内容を正確に反映していることを確認することが価値があります。

次: 小さなCL

小さなCLs

なぜ小さなCLを書くのか？

小さくてシンプルなCLの方が以下のような利点があります：

• 迅速にレビューされる：リビューアは小さなCLを数回に分けて5分ずつ見つける方が、1つの大きなCLを30分のブロックに割り当てるよりも簡単です。
• 徹底的にレビューされる：大きな変更では、詳細なコメントが行ったり来たりすることで、リビューアや著者はイライラし、重要なポイントが見落とされたり落とされたりすることがあります。
• バグを導入する可能性が低い：変更が少ないため、CLの影響を効果的に推論し、バグが導入されていないかを確認することが容易です。
• 却下された場合の無駄な作業が少ない：巨大なCLを書いて、リビューアが全体的な方向性が間違っていると言った場合、多くの作業が無駄になります。
• マージが容易：大きなCLで作業するのは時間がかかるため、マージする際に多くの競合が発生し、頻繁にマージする必要があります。
• 設計が容易：小さな変更の設計とコードの品質を磨く方が、大きな変更の詳細を洗練するよりも簡単です。
• レビューにブロックされることが少ない：全体的な変更の自己完結した部分を送信することで、現在のCLがレビューされるのを待ちながらコーディングを続けることができます。
• ロールバックが簡単：大きなCLは、初期のCLの提出とロールバックCLの間に更新されるファイルに触れる可能性が高く、ロールバックが複雑になります（中間のCLもロールバックする必要があるかもしれません）。

なお、**リビューアはCLが大きすぎるために変更を拒否する権限を持っています。**通常、彼らはあなたの貢献に感謝し、それを一連の小さな変更にするように要求するでしょう。すでに書いた変更を分割するのは大変な作業であり、また、レビュアーが受け入れるべき理由について議論する時間がかかるかもしれません。

Smallとは何ですか？

一般的に、CLの適切なサイズは1つの自己完結した変更です。つまり、以下のような特徴を持ちます。

• CLは、1つのことに対処する最小限の変更を行います。通常、機能全体ではなく、機能の一部に対して行われます。一般的には、CLが大きすぎるよりも小さすぎるCLを書く方が良いです。受託者と協力して、受け入れ可能なサイズを見つけましょう。
• CLには、関連するテストコードが含まれている必要があります。
• CLについて理解するためにリビューアが必要とする情報は、CL自体、CLの説明、既存のコードベース、または既にレビューされたCLに含まれています。
• CLがチェックインされた後も、システムはユーザーと開発者の両方にとってうまく機能し続けます。
• CLの影響が理解しにくいほど小さすぎるわけではありません。新しいAPIを追加する場合は、同じCL内でAPIの使用例を含めることで、リビューアがAPIの使用方法をより理解しやすくすることができます。これにより、未使用のAPIをチェックインすることも防げます。

「大きすぎる」とは具体的な基準はありませんが、通常、100行程度のCLは適切なサイズであり、1000行は通常、大きすぎるとされます。ただし、これはリビューアの判断に委ねられます。変更が広範囲にわたるファイル数も「サイズ」に影響を与えます。1つのファイルで200行の変更は問題ないかもしれませんが、50のファイルに分散されると通常、大きすぎるとされます。

自分がコードを書き始めた瞬間からコードに深く関与しているかもしれませんが、レビュアーにはその文脈がないことを念頭に置いてください。あなたにとっては受け入れ可能なサイズのCLでも、リビューアにとっては圧倒的なものに見えるかもしれません。迷った場合は、思ったよりも小さなCLを書くようにしましょう。リビューアは、あまり小さすぎるCLを受け取ったとは滅多に文句を言いません。

大規模な変更が許容される場合

大規模な変更が問題にならない場合はいくつかあります。

• ファイル全体の削除は、リビューアがレビューするのにあまり時間がかからないため、1行の変更として数えることができます。
• 時には、完全に信頼できる自動リファクタリングツールによって生成された大規模な変更があります。リビューアの仕事は、変更が本当に必要であることを確認し、承認することです。これらの変更は大きくなることがありますが、上記の注意事項（マージやテストなど）は依然として適用されます。

効率的に小さなCLを書く方法

もし小さなCLを書いてからレビュワーに承認されるのを待ってから次のCLを書くとすると、多くの時間を無駄にすることになります。ですので、レビュー待ちの間にブロックされない方法を見つける必要があります。これには、同時に複数のプロジェクトに取り組む、即座に利用可能なレビュワーを見つける、対面でのレビュー、ペアプログラミング、またはCLを分割して即座に作業を続ける方法などが含まれるかもしれません。

CLの分割

相互に依存関係がある可能性のある複数のCLを持つ作業を開始する際には、コーディングに入る前に、高レベルでCLを分割して組織する方法を考えることがしばしば役立ちます。

CLの作成者として、CLを管理・組織するためにも便利ですし、コードリビューアにとっても作業を容易にし、コードレビューを効率化することができます。

以下に、作業を異なるCLに分割するためのいくつかの戦略を示します。

重ねて変更を積み重ねる

自分自身をブロックせずにCLを分割する方法の一つは、小さなCLを書いて、レビューに送り、すぐに最初のCLを基にした別のCLを書き始めることです。ほとんどのバージョン管理システムは、何らかの方法でこれを行うことができます。

ファイルごとの分割

CLを分割する別の方法は、異なるレビュワーが必要ながらもそれ以外は独立した変更を必要とするファイルのグループによって行うことです。

例えば、プロトコルバッファの修正のための1つのCLと、そのプロトを使用するコードの変更のための別のCLを送信します。コードのCLを送信する前にプロトのCLを提出する必要がありますが、両方のCLは同時にレビューできます。これを行う場合、自分が書いた他のCLについて両方のレビュワーに通知することが望ましいかもしれません。これにより、変更の文脈を持っていることができます。

別の例として、コードの変更のための1つのCLと、そのコードを使用する設定や実験のための別のCLを送信します。必要に応じて、設定/実験ファイルはコードの変更よりも早く本番環境にプッシュされることがあるため、必要に応じて簡単に元に戻すこともできます。

水平分割

テックスタックの各層間の変更を分離するのに役立つ共有コードやスタブを作成することを考えてみてください。これにより、開発を迅速化するだけでなく、各層間の抽象化も促進されます。

例えば、クライアント、API、サービス、データモデルの各層を持つ電卓アプリを作成したとします。共有のプロト署名を使用することで、サービスとデータモデルの層を互いに分離することができます。同様に、APIスタブを使用することで、クライアントコードとサービスコードの実装を分割し、それぞれが独立して進めるようにすることができます。同様のアイデアは、より詳細な関数やクラスレベルの抽象化にも適用することができます。

垂直に分割する

階層的で水平なアプローチとは異なり、コードを小さなフルスタックの垂直な機能に分割することもできます。これらの機能のそれぞれは独立した並行実装トラックとなります。これにより、一部のトラックがレビューやフィードバック待ちの間も前進することができます。

水平に分割するでの計算機の例に戻りましょう。今度は、乗算や除算などの新しい演算子をサポートしたいとします。これを、乗算と除算を別々の垂直な機能やサブフィーチャーとして実装することで分割することができます。ただし、共有のボタンスタイリングや共有のバリデーションロジックなど、一部の重複があるかもしれません。

水平および垂直に分割する

さらに進んで、これらのアプローチを組み合わせて、次のような実装計画を作成することができます。各セルは独立したCLです。 モデル（一番下）からクライアントまで進めるようにします。

(訳注)
公式の表が崩れている。
https://google.github.io/eng-practices/review/developer/small-cls.html#splitting-grid
githubの状態に合わせている。
https://github.com/google/eng-practices/blob/master/review/developer/small-cls.md#splitting-horizontally--vertically-splitting-grid

リファクタリングを分離する

リファクタリングは、通常、機能の変更やバグ修正とは別のCLで行うのが最善です。例えば、クラスの移動や名前の変更は、そのクラスのバグ修正とは別のCLで行うべきです。各CLごとに導入される変更をレビュワーが理解しやすくするためには、分離する方がはるかに簡単です。

ただし、ローカル変数名の修正などの小さなクリーンアップは、機能の変更やバグ修正のCL内に含めることもできます。ただし、現在のCLに含めるとレビューがより困難になると判断される場合は、開発者とレビュワーの判断に委ねられます。

関連するテストコードを同じCLにまとめる

CLには関連するテストコードを含めるべきです。ここでの「小ささ」とは、CLが焦点を絞っていることを指し、行数の単純な関数ではありません。

すべてのGoogleの変更にはテストが必要です。

ロジックを追加または変更するCLには、新しい動作に対応する新しいまたは更新されたテストが必要です。純粋なリファクタリングCL（動作を変更する意図のないもの）もテストでカバーする必要があります。理想的には、これらのテストはすでに存在しているはずですが、存在しない場合は追加する必要があります。

独立したテストの変更は、まず別々のCLに入れることができます。リファクタリングのガイドラインと同様です。これには以下が含まれます：

• 既存の提出済みコードを新しいテストで検証すること。
  • 重要なロジックがテストでカバーされていることを確認します。
  • 影響を受けるコードの後続のリファクタリングに対する信頼性を高めます。たとえば、テストされていないコードをリファクタリングしたい場合、リファクタリングCLを提出する前にテストCLを提出することで、リファクタリング前後でテストされた動作が変わらないことを検証できます。
• テストコードのリファクタリング（例：ヘルパー関数の導入）。
• 大規模なテストフレームワークコードの導入（例：統合テスト）。

ビルドを壊さないでください

もし複数のCLが互いに依存している場合、各CLが提出された後もシステム全体が正常に動作し続けるようにする必要があります。そうしないと、CLを提出するたびに他の開発者全員のビルドが数分間（もしくは後のCLの提出で何か予期せぬ問題が発生した場合はそれ以上の時間）壊れてしまう可能性があります。

十分に小さくすることはできません

CLが大きくならざるを得ない状況に遭遇することがあります。しかし、これは非常に稀です。小さなCLを書くことに慣れた著者は、ほとんどの場合、機能を一連の小さな変更に分解する方法を見つけることができます。

大きなCLを書く前に、リファクタリング専用のCLを前に置くことで、よりクリーンな実装の道を開くことができるかどうかを考えてみてください。チームメンバーと話し合い、機能を小さなCLに実装する方法についての意見を聞いてみてください。

これらのオプションがすべて失敗した場合（非常に稀ですが）、事前にレビュワーの承諾を得て大きなCLをレビューするようにしてください。これにより、彼らは何が来るかについて警告されます。この状況では、長い時間をかけてレビュープロセスを進めることを予想し、バグを導入しないように注意し、テストの作成に特に注意してください。

次: レビューコメントの処理方法

レビューコメントの扱い方

CL（変更リスト）をレビューに送信すると、リビューアから複数のコメントが返ってくることがあります。以下に、レビューコメントの扱いについて知っておくと役立つ情報をいくつか紹介します。

個人的に受け取らないでください

レビューの目的は、コードベースと製品の品質を維持することです。 リビューアがあなたのコードに対して批判を行うときは、それをあなたやあなたの能力に対する個人攻撃ではなく、あなたやコードベース、Googleを助けるための試みと考えてください。

時にはリビューアはイライラしており、コメントでそのイライラを表現することがあります。これはリビューアとしては良い慣行ではありませんが、開発者としてはそれに備えておくべきです。自分に対してリビューアが何を伝えようとしているのか、"リビューアが実際に言っていること"として行動してください。

コードレビューコメントに対して怒りで応答しないでください。 それはプロのエチケットの重大な違反であり、コードレビューツールに永遠に残ります。もし怒りやイライラで親切に応答することができない場合は、しばらくコンピュータから離れるか、他の作業に取り組んで落ち着くまで待ってから丁寧に返信してください。

一般的に、リビューアが建設的で礼儀正しい方法でフィードバックを提供していない場合は、直接話し合うことを説明してください。もし直接話すことができない場合やビデオ通話で話すことができない場合は、プライベートなメールで彼らに説明してください。嫌な点や異なる方法をお願いすることを親切に説明してください。もし彼らがこのプライベートな話し合いにも建設的でないように応答したり、意図した効果が得られない場合は、適切な場合には上司にエスカレーションしてください。

コードの修正

もしリビューアがあなたのコードの何かを理解できないと言った場合、最初にするべきはコード自体を明確にすることです。もしコードを明確にすることができない場合、コードの存在理由を説明するコメントを追加してください。コメントが無意味に思える場合にのみ、コードレビューツールでの説明が適切な対応です。

もしリビューアがあなたのコードの一部を理解できなかった場合、将来のコード読者も同様に理解できない可能性があります。コードレビューツールでの返答は将来のコード読者には役立ちませんが、コードを明確にするかコメントを追加することは役立ちます。

協力的に考える

CLを書くのは大変な作業です。レビューのために送信し、完成したと感じ、さらなる作業は必要ないと確信することは、しばしば非常に満足感を得ることです。特に、自分が意見に同意しない場合、変更を求めるコメントを受け取ることは、イライラすることがあります。

このような時には、一歩引いて考えてみてください。リビューアがコードベースやGoogleに役立つ有益なフィードバックを提供しているかどうかを考えてみてください。自分自身に最初に問いかけるべき質問は常に「リビューアが何を求めているのか理解できているか？」です。

その質問に答えられない場合は、リビューアに明確化を求めてください。

そして、コメントを理解しているが、それに同意しない場合は、協力的に考えることが重要です。攻撃的な態度や防御的な態度ではなく、協力的に考えることが重要です。

悪い例: 「いや、それはやらない。」

良い例: 「私はXを選んだ理由は、[これらの利点/欠点]と[これらのトレードオフ]です。私の理解では、Yを使用すると[これらの理由]で悪化すると考えています。Yが元のトレードオフにより適しているということですか？トレードオフを異なる視点で考慮すべきですか？それとも他の何かですか？」

覚えておいてください、常に 礼儀と尊重が最優先 です。リビューアと意見が異なる場合でも、協力する方法を見つけてください：明確化を求める、利点と欠点を議論する、コードベース、ユーザー、またはGoogleにとって自分の方法がより良い理由を説明するなど。

時には、リビューアが知らないユーザーやコードベース、またはCLについての情報を持っているかもしれません。適切な場所でコードを修正し、リビューアとの議論に参加し、より多くの文脈を提供してください。通常、技術的な事実に基づいて、自分とリビューアの間で合意に達することができます。

紛争の解決

紛争を解決するための最初のステップは、常にリビューアとの合意を図ることです。合意に達しない場合は、コードレビューの基準を参照してください。この基準では、そのような状況で従うべき原則が示されています。

どのようにコードリビューするか

このセクションのページでは、長年の経験に基づいて、コードレビューの最良の方法に関する推奨事項が記載されています。これらはすべて、1つの完全なドキュメントを多くのセクションに分割したものです。すべてを読む必要はありませんが、多くの人々が、自分自身やチームにとって非常に役立つと感じています。

• コードレビューの標準
• コードレビューで探すべきもの
• レビュー中のCLのナビゲーション
• コードレビューのスピード
• コードレビューコメントの書き方
• コードレビューでの反対意見の扱い方

また、CL作成者ガイドも参照してください。これは、CLがレビューを受けている開発者に詳細なガイダンスを提供しています。

コードレビューの基準

コードレビューの主な目的は、Googleのコードベースの全体的なコードの健全性が時間とともに向上していることを確認することです。コードレビューのすべてのツールとプロセスは、この目的を達成するために設計されています。

これを達成するためには、いくつかのトレードオフをバランスさせる必要があります。

まず、開発者は自分のタスクを進めることができなければなりません。コードベースに改善を提出しなければ、コードベースは改善されません。また、レビュアが変更を非常に難しくすると、開発者は将来の改善をする意欲を失います。

一方、レビュアの責任は、各CLがコードベースの全体的なコードの健全性が時間とともに低下しないようにすることです。これは難しいことです。なぜなら、コードベースはしばしば時間とともにコードの健全性がわずかに低下することで劣化するからです。特に、チームが時間的制約の下にあり、目標を達成するために手を抜かざるを得ないと感じている場合には、さらにそうなります。

また、レビュアは自分がレビューしているコードに所有権と責任を持っています。彼らはコードベースが一貫性を保ち、保守可能であり、"コードレビューで探すべきもの"で言及されている他のすべての要素を確保したいと考えています。

したがって、コードレビューで期待される標準として次のルールが得られます。

一般的に、CLがシステム全体のコードの健全性を確実に向上させる状態になった時点で、レビュアはCLの承認を優先すべきです。CLが完璧でなくても構いません。

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

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

ここで重要なポイントは、「完璧な」コードというものは存在しないということです。ただし、より良いコードは存在します。レビュアは、著者に対してCLのすべての細部を磨くことを要求するべきではありません。代わりに、レビュアは前進する必要性と提案されている変更の重要性をバランスさせるべきです。完璧さを求めるのではなく、レビュアが求めるべきは「継続的な改善」です。システムの保守性、可読性、理解性を全体的に向上させるCLは、数日間遅延させるべきではありません。

リビューアは、いつでも何かが改善できるというコメントを自由に残すことができるが、それが非常に重要でない場合は、「Nit: 」のような接頭辞を付けて、著者にそれを無視してもよいと伝えることができます。

注意：このドキュメントには、システム全体のコードの健全性を明らかに悪化させるようなCLをチェックインすることを正当化するものはありません。それを行うのは、緊急時のみです。

メンタリング

コードレビューは、開発者に言語、フレームワーク、または一般的なソフトウェア設計原則について新しいことを教える重要な機能を持つことがあります。開発者が新しいことを学ぶのに役立つコメントを残すことは常に良いことです。知識を共有することは、システムのコード品質を時間とともに向上させる一部です。ただし、コメントが純粋に教育的であり、このドキュメントで説明されている基準を満たすために必須ではない場合は、「Nit: 」という接頭辞を付けるか、それが著者が解決する必要がないことを示す他の方法を使用してください。

原則

• 技術的な事実とデータは、意見や個人の好みよりも優先される。

• スタイルに関する問題では、スタイルガイドが絶対的な権威である。スタイルガイドにない純粋なスタイルのポイント（空白など）は、個人の好みの問題である。スタイルは、既存のスタイルと一貫性があるべきである。既存のスタイルがない場合は、著者のスタイルを受け入れる。

• **ソフトウェア設計の側面は、ほとんど常に純粋なスタイルの問題や個人の好みだけではありません。**それらは基本原則に基づいており、その原則に基づいて評価するべきです。時にはいくつかの妥当な選択肢があります。著者が（データまたは堅実なエンジニアリング原則に基づいて）いくつかのアプローチが同じくらい妥当であることを示すことができれば、リビューアは著者の好みを受け入れるべきです。そうでない場合は、ソフトウェア設計の標準的な原則によって選択されます。

• 他のルールが適用されない場合、リビューアは、システム全体のコードの健全性を悪化させない限り、現在のコードベースに一貫性を求めることができます。

紛争の解決

コードレビューにおけるいかなる紛争においても、最初のステップは常に開発者とレビュアーがこのドキュメントや他のドキュメント（CL作成者ガイドおよびリビューアガイド）の内容に基づいて合意を図ることです。

合意に達することが特に困難な場合、コードレビューコメントだけで紛争を解決しようとする代わりに、リビューアと作成者の間で対面会議やビデオ会議を行うことが役立つ場合があります。（ただし、その場合は議論の結果をCLのコメントとして記録しておくことを忘れないでください。将来の読者のために。）

それでも状況が解決しない場合、最も一般的な解決方法はエスカレーションです。しばしばエスカレーションの経路は、より広範なチームの議論、テクニカルリードの意見、コードのメンテナからの決定の要求、またはエンジニアリングマネージャーの助けを求めることです。作者とリビューアが合意に達しないためにCLを放置しないでください。

次: コードレビューで探すべきポイント

コードレビューで注目すべきポイント

注意：これらのポイントを考慮する際には、常にコードレビューの標準を参考にしてください。

デザイン

レビューで最も重要なのは、CLの全体的なデザインです。 CL内のさまざまなコードの相互作用は意味をなしていますか？この変更は、コードベースではなくライブラリに属していますか？システム全体との統合はうまくいっていますか？この機能を追加するのに適切な時期ですか？

機能性

このCLは、開発者が意図した通りの動作をしていますか？開発者が意図したことは、このコードのユーザーにとって良いものですか？「ユーザー」とは、通常、エンドユーザー（変更の影響を受ける場合）と開発者（将来このコードを「使用」する必要がある）の両方を指します。

ほとんどの場合、開発者はCLを十分にテストして、コードレビューに到達するまでに正しく動作するようにすることが期待されます。ただし、リビューアとしては、エッジケースを考え、並行性の問題を探し、ユーザーのように考え、コードを読むだけで見えるバグがないか確認する必要があります。

CLの動作を検証することもできますが、リビューアがCLの動作をチェックするのは、UIの変更など、ユーザーに影響を与える変更の場合が最も重要です。コードを読むだけでは、どのような変更がユーザーにどのような影響を与えるかを理解するのは難しいです。そのような変更の場合、開発者にデモをしてもらうか、CLにパッチを当てて試すことができない場合は、機能のデモをしてもらうことができます。

また、コードレビュー中に特に機能性について考える必要があるのは、CLに理論的にデッドロックや競合状態を引き起こす可能性のある並行プログラミングが行われている場合です。これらの問題は、コードを実行するだけでは検出するのが非常に難しく、問題が導入されていないことを確認するために、開発者とリビューアの両方が注意深く考える必要があります。（競合状態やデッドロックが発生する可能性のある並行性モデルを使用しないことは、コードレビューやコードの理解を非常に複雑にする可能性があるため、非常に重要な理由です。）

複雑さ

CLは必要以上に複雑ですか？CLの各レベルで確認してください。個々の行は複雑すぎますか？関数は複雑すぎますか？クラスは複雑すぎますか？「複雑すぎる」とは通常、「コード読み手がすぐに理解できない」という意味です。また、「開発者がこのコードを呼び出したり変更しようとすると、バグを発生させる可能性が高い」という意味もあります。

特定のタイプの複雑さは「過剰設計」です。開発者がコードを必要以上に一般化したり、現在のシステムで必要のない機能を追加したりしている場合です。リビューアは特に過剰設計に対して警戒すべきです。開発者に対して、現在解決する必要のある問題を解決するように促し、将来解決するかもしれない問題ではなく、現在の問題を解決するようにしましょう。将来の問題は、実際の形状と要件を物理的な宇宙で確認できるようになった時に解決すべきです。

テスト

変更に適した単体テスト、統合テスト、エンドツーエンドテストを要求してください。一般的には、テストは本番コードと同じCLに追加する必要がありますが、CLが緊急事態を処理している場合を除きます。

CL内のテストが正確で、意味のある、有用なものであることを確認してください。テストは自己テストを行いませんし、テストのためにテストを書くことはほとんどありません。人間がテストが有効であることを確認する必要があります。

コードが壊れたときにテストは実際に失敗しますか？コードが変更された場合、テストは誤った結果を出力し始めますか？各テストは単純で有用なアサーションを行っていますか？テストは異なるテストメソッド間で適切に分離されていますか？

テストもメンテナンスが必要なコードですので、メインのバイナリの一部ではないためにテストに複雑さを受け入れないでください。

名前付け

開発者はすべての要素に適切な名前を選びましたか？適切な名前は、アイテムの内容や機能を完全に伝えるのに十分な長さであり、読みにくくなるほど長くなっていませんか？

コメント

開発者は理解しやすい英語で明確なコメントを書いていますか？すべてのコメントが実際に必要ですか？通常、コメントはコードが存在する理由を説明するために使用され、コードが何をしているかを説明するためには使用されません。コード自体が十分に明確でない場合は、コードをよりシンプルにする必要があります。ただし、正規表現や複雑なアルゴリズムなどの場合は、自身のコードに含まれていない情報を説明するコメントが非常に役立つことがあります。ただし、ほとんどの場合、コメントは意思決定の背後にある理由など、コード自体に含まれない情報のために使用されます。

また、この変更リストの前にあったコメントも参考になる場合があります。今は削除できるTODOコメントや、この変更を行わないように忠告するコメントなどがあるかもしれません。

コメントは、クラス、モジュール、または関数のドキュメントとは異なり、コードの目的、使用方法、使用時の動作などを表現するべきです。

スタイル

Googleでは、主要な言語だけでなく、ほとんどのマイナーな言語に対してもスタイルガイドがあります。適切なスタイルガイドに従ってCLを作成してください。

スタイルガイドにないスタイルの改善をしたい場合は、「Nit:」という接頭辞をコメントに付けて、開発者にそれがコードを改善するための細かい指摘であることを伝えてください。個人的なスタイルの好みだけでCLの提出をブロックしないでください。

CLの作者は、主要なスタイルの変更を他の変更と組み合わせてはいけません。CLで何が変更されているかがわかりにくくなり、マージやロールバックが複雑になり、他の問題を引き起こします。たとえば、ファイル全体の書式を変更したい場合は、まず書式変更のみを含んだ別のCLを送り、その後に機能的な変更を含んだ別のCLを送ってください。

一貫性

既存のコードがスタイルガイドと一貫していない場合はどうなりますか？コードレビューの原則によれば、スタイルガイドが絶対的な権威です。つまり、スタイルガイドで要求されているものは、CL（変更リスト）はガイドラインに従うべきです。

一部の場合、スタイルガイドは要件ではなく、推奨事項を示しています。これらの場合、新しいコードが推奨事項に従うべきか、周囲のコードに一貫性を持たせるべきかは判断に委ねられます。ローカルな一貫性が混乱を招く場合を除き、スタイルガイドに従うことを優先しましょう。

他のルールが適用されない場合は、著者は既存のコードと一貫性を保つべきです。

いずれの場合でも、著者に対してバグの報告と既存のコードの整理のためのTODOの追加を促してください。

ドキュメンテーション

もしCLがユーザーがコードをビルド、テスト、操作、リリースする方法に変更を加える場合、関連するドキュメンテーション（README、g3docページ、生成されたリファレンスドキュメントなど）も更新されているか確認してください。もしCLがコードを削除または非推奨化する場合、ドキュメンテーションも削除すべきかどうか考えてください。 もしドキュメンテーションが不足している場合は、それを要求してください。

すべての行

一般的な場合、レビューすることに割り当てられた_すべて_のコードの行を見てください。データファイルや生成されたコード、大きなデータ構造などは、時々スキャンすることができますが、人間が書いたクラス、関数、またはコードブロックをスキャンして、その中身が問題ないと仮定しないでください。 明らかに、あるコードは他のコードよりも注意深く調査する価値がありますが、それはあなたが判断しなければならないことです。少なくとも、すべてのコードが何をしているのかを_理解_していることを確認してください。

もしコードを読むのが難しく、それがレビューを遅らせている場合は、開発者にそれを伝えて、レビューする前にそれを明確にしてもらうように待ってください。Googleでは、優れたソフトウェアエンジニアを採用しています。あなたもその一人です。もしコードが理解できない場合、他の開発者も理解できない可能性が非常に高いです。ですので、開発者にそれを明確にしてもらうことで、将来の開発者がこのコードを理解するのを助けることにもなります。

もしコードが理解できるけれども、レビューの一部を適任と感じない場合は、レビュアーがいることを確認してください。特にプライバシーやセキュリティ、並行性、アクセシビリティ、国際化などの複雑な問題については、適任なリビューアがいることを確認してください。

例外

もし、すべての行をレビューすることが意味をなさない場合はどうなりますか？ 例えば、あなたがCLの複数のレビュワーのうちの一人であり、次のような要求がある場合です。

• 大きな変更の一部である特定のファイルのみをレビューするように求められる場合。
• 高レベルの設計、プライバシーやセキュリティの影響など、CLの特定の側面のみをレビューするように求められる場合。

これらの場合、レビューした部分をコメントで明記してください。コメント付きのLGTMを優先してください。

もし、他のレビュワーがCLの他の部分をレビューしたことを確認した後にLGTMを与えたい場合は、それを明示的にコメントで記述して、期待値を設定してください。CLが望ましい状態に達したら、迅速に応答することを目指してください。

文脈

CLを広い文脈で見ることは、しばしば役立ちます。通常、コードレビューツールは、変更される部分の周りの数行のコードしか表示しません。実際に変更が意味をなしているかどうかを確認するには、ファイル全体を見る必要があることもあります。たとえば、追加されるのはわずか4行のコードだけかもしれませんが、ファイル全体を見ると、その4行が50行のメソッドにあることがわかり、このメソッドをより小さなメソッドに分割する必要があることがわかります。

また、CLをシステム全体の文脈で考えることも有用です。このCLはシステムのコードの健全性を向上させているのか、システム全体をより複雑にしたり、テストが少なくなったりしているのか、などを考えると良いでしょう。**システムのコードの健全性を損なうCLは受け入れないでください。**ほとんどのシステムは、多くの小さな変更を通じて複雑になっていくため、新しい変更においても小さな複雑さを防ぐことが重要です。

良いこと

もしCLで素敵なものを見つけたら、開発者に伝えてください。特に、彼らがあなたのコメントに素晴らしい方法で対応した場合は。コードレビューはしばしばミスに焦点を当てますが、良い実践に対する励ましと感謝も提供すべきです。開発者に何が間違っているかを伝えるよりも、彼らが何を正しく行ったかを伝える方が、メンタリングの観点からはさらに価値があります。

概要

コードレビューを行う際には、以下の点に注意してください：

• コードがよく設計されていること。
• 機能がコードのユーザーにとって良いものであること。
• UIの変更が合理的で見栄えが良いこと。
• 並行プログラミングが安全に行われていること。
• コードが必要以上に複雑でないこと。
• 開発者が将来必要になるかもしれないものを実装していないこと。
• コードに適切な単体テストがあること。
• テストがよく設計されていること。
• 開発者がすべての要素にわかりやすい名前を付けていること。
• コメントが明確で有用であり、主に「なぜ」を説明していること。
• コードが適切にドキュメント化されていること（一般的にはg3docで）。
• コードがスタイルガイドに準拠していること。

レビューを依頼されたコードのすべての行を確認し、文脈を考慮し、コードの健全性を向上させ、開発者が行った良い点を褒めるようにしてください。

次: レビュー中のCLのナビゲーション

レビュー中のCLのナビゲーション

概要

これで何を探すべきかがわかりました。それでは、複数のファイルにまたがるレビューを最も効率的に管理する方法は何でしょうか？

1. 変更内容は理解できますか？説明は適切ですか？
2. 変更の最も重要な部分を最初に見てください。全体的には設計が良いですか？
3. 適切な順序でCLの残りの部分を見てください。

ステップ1: 変更の広い視点を持つ

まず、CLの説明を見て、このCLが一般的に意味をなしているかどうかを確認してください。この変更が最初から行われるべきではなかった場合は、なぜこの変更が行われるべきではないのかについてすぐに説明してください。このような変更を拒否する際には、開発者に代わりに何をすべきかを提案することも良いアイデアです。

例えば、「この変更には良い仕事がされているようですね、ありがとうございます！ただし、私たちは実際にはここで修正しているFooWidgetシステムを削除する方向に進んでいるため、現在それに新たな修正を加えたくありません。代わりに、新しいBarWidgetクラスをリファクタリングすることはいかがでしょうか？」

リビューアは現在のCLを拒否し、代わりの提案を行っただけでなく、それを_礼儀正しく_行いました。このような礼儀正しさは重要です。なぜなら、私たちは意見が異なる場合でも、開発者として互いを尊重することを示したいからです。

もし、望ましくない変更を表すCLが数多くある場合は、チームの開発プロセスや外部貢献者向けの投稿プロセスを見直し、CLが書かれる前により多くのコミュニケーションが行われるようにすることを検討してください。人々に「いいえ」と伝えることは、彼らが大量の作業を行い、それを捨てるか大幅に書き直さなければならなくなる前に伝える方が良いです。

ステップ2: CLの主要な部分を調べる

このCLの「主要な」部分となるファイルまたはファイルを見つけてください。通常、論理的な変更が最も多いファイルがあり、それがCLの主要な部分です。まずはこれらの主要な部分を見てください。これにより、CLのすべての小さな部分の文脈がわかり、コードレビューのスピードが向上します。CLが大きすぎて、どの部分が主要な部分かわからない場合は、開発者に最初に何を見るべきか尋ねるか、CLを複数のCLに分割する.ように依頼してください。

この部分のCLに重大な設計上の問題が見つかった場合は、すぐにそれらのコメントを送信してください。今すぐCLの残りをレビューする時間がなくても、重要な設計上の問題がある場合は、CLの残りのコードをレビューすることは時間の無駄かもしれません。実際、重大な設計上の問題がある場合、レビュー対象の他のコードの多くは消えて問題にならない可能性があります。

これらの重大な設計上のコメントをすぐに送信することが非常に重要な理由は2つあります：

• 開発者は通常、CLをメールで送信した後、レビューを待ちながら新しい作業をすぐに開始します。レビュー中のCLに重大な設計上の問題がある場合、後のCLの再作業も行わなければなりません。問題のある設計の上に余分な作業を行う前に、それらを見つける必要があります。
• 大規模な設計変更は、小規模な変更よりも時間がかかります。開発者はほぼすべての締め切りを持っています。それらの締め切りを守りながら、コードベースに品質の高いコードを残すためには、開発者はCLの重大な再作業をできるだけ早く開始する必要があります。

ステップ3: 適切な順序でCLの残りを確認する

CL全体に大きな設計上の問題がないことを確認したら、各ファイルを見逃さずにレビューするための論理的な順序を考えてみてください。通常、主要なファイルを見た後は、コードレビューツールが提示する順序で各ファイルを見るのが最も簡単です。時には、メインのコードを読む前にテストを先に読むと、変更が何をするものかのアイデアが得られることもあります。

次: コードレビューの速度

コードレビューの速度

なぜコードレビューを迅速に行うべきか

Googleでは、開発者チームが一緒に製品を生産する速度を最適化します。これは、個々の開発者がコードを書く速度を最適化することとは対照的です。個々の開発速度も重要ですが、チーム全体の速度ほどではありません。

コードレビューが遅いと、いくつかの問題が生じます：

• チーム全体の速度が低下します。 レビューにすぐに対応しない個人は他の仕事を進めるかもしれません。しかし、チームの残りの部分にとっての新機能やバグ修正は、各CLがレビューと再レビューを待つ間、日、週、または月単位で遅れます。
• 開発者がコードレビュープロセスに抗議し始めます。 リビューアが数日おきにしか反応せず、その都度CLに大きな変更を要求する場合、それは開発者にとってイライラすることであり、困難です。しばしば、これは「リビューアが厳しすぎる」という不満として表現されます。リビューアが開発者が更新を行うたびに_迅速に_同じ重要な変更（実際にコードの健全性を向上させる変更）を要求する場合、不満は消える傾向があります。コードレビュープロセスに関するほとんどの不満は、プロセスを迅速にすることで実際に解決されます。
• コードの健全性に影響を与える可能性があります。 レビューが遅いと、開発者がそれほど良くないCLを提出する圧力が増します。遅いレビューは、コードのクリーンアップ、リファクタリング、および既存のCLへのさらなる改善を抑制する傾向があります。

コードレビューはどれくらいの速さであるべきか

集中しているタスクの最中でない限り、コードレビューは受け取った直後に行うべきです。

コードレビュー要求に対して反応する最大時間は1営業日です（つまり、翌朝の最初の時間）。

これらのガイドラインに従うと、典型的なCLは必要に応じて1日以内に複数ラウンドのレビューを受けることになります。

速度 vs. 中断

個人の作業速度がチームの速度を上回るべき唯一の時があります。コードを書くなど、集中している作業の最中に、自分を中断してコードレビューを行うべきではありません。 研究によると、中断された後に開発のスムーズな流れに戻るまでには、開発者に長い時間がかかることが示されています。従って、コーディング中に自分自身を中断することは、他の開発者がコードレビューを少し待たされることよりも、実際にはチームにとって_より_コストがかかることになります。

代わりに、作業の中断点でレビューの依頼に応答することを待ちます。これは、現在のコーディングタスクが完了した時、ランチ後、会議から戻った時、休憩室から戻ってきた時などが該当します。

迅速な対応

コードレビューの速度について話す際、私たちが関心を持っているのは、CLがレビューを通過して提出されるまでにかかる時間ではなく、_対応_の時間です。全プロセスも理想的には速いべきですが、個々の対応が迅速に行われることの方が、全プロセスが迅速に行われることよりもさらに重要です。

たとえ全体のレビュー_プロセス_を通過するのに時々長い時間がかかる場合でも、プロセス全体を通じてリビューアからの迅速な対応があることで、「遅い」と感じる開発者のフラストレーションが大幅に軽減されます。

CLが到着した時に完全なレビューを行うのにあまりにも忙しい場合でも、いつ対応できるか開発者に知らせる迅速な返答を送ったり、より迅速に対応できる可能性のある他のリビューアを提案したり、いくつかの初期の広範なコメントを提供することができます。（注意：これは、このような返答を送るためにさえコーディングを中断すべきだという意味ではありません。作業の合理的な中断点で返答を送ってください。）

リビューアがレビューに十分な時間を費やし、「LGTM」が「このコードは私たちの基準を満たしている」ということを確信していることが重要です。 しかし、個々の対応は理想的には迅速であるべきです。

時差を考慮したレビュー

時差の差を扱う際は、彼らがその日の業務時間内に返答できるように、著者に返答しようと努めてください。彼らがすでにその日の業務を終えていた場合は、彼らが翌日の業務を開始する前にレビューを完了させるようにしてください。

コメント付きLGTM

コードレビューを迅速に進めるため、リビューアが未解決のコメントを残しつつもLGTM/承認を出すべき特定の状況があります。これは以下のいずれかの場合に行われます：

• リビューアが開発者が全ての残りのコメントを適切に対処すると確信している場合。
• 残りの変更が些細で、開発者が必ずしも行う必要はない場合。

リビューアは、どちらのオプションを意図しているかを明確に指定すべきです（それが明確でない場合）。

開発者とリビューアが異なるタイムゾーンにいて、開発者が「LGTM、承認」を得るために1日待たなければならない場合には、特にコメント付きLGTMを検討する価値があります。

大きなCL

あなたにとても大きなコードレビューが送られてきて、いつレビューできるか分からない場合、通常の対応は開発者にCLをいくつかの小さなCLに分割するよう依頼することです。これは、開発者に追加の作業が必要であっても、通常は可能であり、リビューアにとって非常に役立ちます。

CLを小さなCLに分割_できない_場合、そしてあなたがすぐに全体をレビューする時間がない場合は、少なくともCLの全体的な設計についていくつかのコメントを書き、改善のために開発者に返送してください。リビューアとしてのあなたの目標の1つは、コードの健全性を犠牲にすることなく、常に開発者のブロックを解除するか、迅速に何らかの行動を取ることができるようにすることです。

時間と共に改善されるコードレビュー

これらのガイドラインに従い、コードレビューに厳格であれば、コードレビュープロセス全体が時間とともにますます速くなることがわかるでしょう。開発者は健全なコードに必要なことを学び、最初から素晴らしいCLを送ってくるようになり、レビュー時間がますます少なくなります。リビューアは迅速に対応し、レビュープロセスに不必要な遅延を加えないように学びます。 しかし、速度の想像される改善のためにコードレビュー基準や品質を妥協しないでください—それは実際には何も早く進めることはありません、長い目で見れば。

緊急事態

CLが非常に迅速に_全_レビュープロセスを通過しなければならない緊急事態もあり、その場合は品質ガイドラインが緩和されるかもしれません。しかし、どの状況が実際に緊急事態として資格があるか、そしてどれがそうでないかについての説明は、何が緊急事態か？をご覧ください。

次: コードレビューコメントの書き方

コードレビューコメントの書き方

概要

• 優しくあれ。
• 理由を説明せよ。
• 問題を指摘するだけでなく、明確な指示と開発者が自分で決めることのバランスを取ろう。
• 複雑さを説明するだけでなく、開発者にコードを簡素化したり、コメントを追加するように促そう。

礼儀

一般的に、礼儀正しく尊重することは重要です。同時に、コードをレビューしている開発者に対して明確かつ助けになるコメントをすることも重要です。これを実践する方法の一つは、常にコードについてコメントをすることであり、開発者についてのコメントは避けることです。常にこの慣行に従う必要はありませんが、不快感や論争を引き起こす可能性のあることを言う場合には、特にこの慣行を使用するべきです。例えば：

悪い例: "なぜあなたはここでスレッドを使用したのですか？明らかに並行性から得られる利益はありません。"

良い例: "ここでの並行性モデルは、実際のパフォーマンスの利益が見当たらないまま、システムに複雑さを追加しています。パフォーマンスの利益がないため、このコードは複数のスレッドを使用するのではなく、シングルスレッドであることが最善です。"

なぜ説明するのか

上記の「良い」例について気づくことがあるでしょう。それは、コメントをする理由を開発者に理解させるのに役立つということです。レビューコメントにこの情報を常に含める必要はありませんが、時には意図やベストプラクティス、提案がコードの品質向上にどのように寄与するかについて少し説明することが適切な場合もあります。

ガイダンスの提供

一般的に、CLの修正は開発者の責任であり、リビューアの責任ではありません。 解決策の詳細な設計や開発者のためのコードの記述は必要ありません。

ただし、リビューアは無力ではあってはなりません。一般的には、問題を指摘し、直接的なガイダンスを提供する適切なバランスを取るべきです。問題を指摘し、開発者に決定を任せることは、開発者が学ぶのに役立ち、コードレビューを行いやすくすることができます。また、開発者はリビューアよりもコードに近いため、より良い解決策につながることもあります。

ただし、時には直接的な指示、提案、またはコードの提供の方が役立つこともあります。コードレビューの主な目標は、最も優れたCLを得ることです。第二の目標は、開発者のスキルを向上させ、時間の経過とともにますます少ないレビューが必要となるようにすることです。

人々は、自分がうまくやっていることを強化されることから学びます。ただし、改善できる点だけでなく、CLで好きな点もコメントしてください！例えば、開発者が乱雑なアルゴリズムを整理した、模範的なテストカバレッジを追加した、またはレビュアー自身がCLから何かを学んだなどです。すべてのコメントと同様に、なぜそのような点が好きなのかを含めて、開発者に良い慣行を続けるようにさらに促すことが重要です。

ラベルコメントの重要度

コメントの重要度をラベル付けして、必要な変更とガイドラインや提案を区別することを検討してください。

いくつかの例

Nit: これは小さなことです。技術的にはやるべきですが、大きな影響はありません。

Optional (または Consider): これは良いアイデアかもしれませんが、厳密には必要ではありません。

FYI: このCLではこれをやることは期待していませんが、将来的に考えるのに興味深いかもしれません。

これにより、レビューの意図が明確になり、著者がさまざまなコメントの重要性を優先順位付けするのに役立ちます。また、誤解を避けるのにも役立ちます。たとえば、コメントにラベルがない場合、著者はすべてのコメントを必須と解釈するかもしれませんが、実際には情報提供やオプションのコメントである場合もあります。

説明の受け入れ

もし開発者に理解できないコードの説明を求めると、通常は彼らがコードをより明確に書き直すことになるはずです。 時には、コードにコメントを追加することも適切な対応ですが、それが単に過度に複雑なコードを説明するだけでない限りです。

コードレビューツールだけで書かれた説明は、将来のコード読者にとって役に立ちません。 それらは、通常のコードの読者が既に知っていることを開発者が説明する場合など、一部の状況でのみ受け入れられます。

次: コードレビューでの反対意見の扱い方

コードレビューでの反発の扱い

時には、開発者がコードレビューに反発することがあります。彼らはあなたの提案に同意しないか、一般的にあなたが厳しすぎると不満を述べるかもしれません。

誰が正しいのか？

開発者があなたの提案に反対する場合、まず彼らが正しいかどうかを考えてみてください。彼らはあなたよりもコードに近いことが多いため、特定の側面についてより良い洞察を持っているかもしれません。彼らの主張は理にかなっていますか？コードの健全性の観点からも理にかなっていますか？もしそうなら、彼らに正しいと伝え、問題を解決させましょう。

しかし、開発者が常に正しいわけではありません。この場合、リビューアはなぜ自分の提案が正しいと考えているのか、さらに説明する必要があります。良い説明は、開発者の返信を理解していることと、変更がなぜ要求されているのかについての追加情報を示します。

特に、リビューアが自分の提案がコードの健全性を向上させると信じている場合、結果としてのコード品質の向上が追加の作業を正当化すると考えるなら、変更を推進し続けるべきです。コードの健全性の向上は、小さなステップで行われる傾向があります。

時には、提案を説明するのに数回のやり取りが必要なこともあります。ただし、常に礼儀正しく、開発者に彼らの言っていることを_聞いている_ことを伝え、ただ_同意しない_ことを伝えましょう。

開発者を不快にさせること

リビューアは、開発者が改善を求められると不快になると考えることがあります。開発者は時には不快になることもありますが、それは一時的なものであり、後であなたが彼らのコードの品質向上に役立ったことに非常に感謝します。通常、コメントが礼儀正しい場合、開発者は実際には全く不快にならず、心配はリビューアの心の中にあるだけです。不快感は通常、コメントの書き方よりもコードの品質に対するリビューアの主張に関連しています。

後で片付ける

開発者たちは（当然のことながら）仕事を進めたいと思っているため、抵抗が生じることがよくあります。彼らはこのCLを提出するためにもう一度レビューを受けたくないのです。そのため、後で何かをきれいにすると言って、今すぐこのCLを承認してほしいと言います。一部の開発者はこれに非常にうまく対応し、すぐに問題を修正するための追加のCLを書きます。しかし、経験からわかるように、開発者が元のCLを書いた後に時間が経つほど、このクリーンアップが行われる可能性は低くなります。実際、開発者が現在のCLの直後にクリーンアップを行わない限り、ほとんど行われません。これは開発者が無責任なわけではなく、他の仕事の中でクリーンアップが見失われたり忘れられたりするためです。したがって、コードがコードベースに組み込まれて「完了」する前に、開発者にCLのクリーンアップを行うように強く求めるのが通常最善です。後で「片付ける」ということを許すことは、コードベースが劣化する一般的な方法です。

もしCLが新たな複雑さを導入する場合、緊急事態でない限り、提出前にクリーンアップする必要があります。もしCLが周囲の問題を露呈し、現時点では対処できない場合、開発者はクリーンアップのためのバグを報告し、自分自身に割り当てるべきです。また、コードにTODOコメントを書いて、報告されたバグを参照することもできます。

厳格さに関する一般的な苦情

以前は比較的緩いコードレビューを行っていた場合、厳格なレビューに切り替えると、一部の開発者は非常に大きな苦情を言うことがあります。コードレビューの速度を改善することで、これらの苦情はしだいに薄れていくことがあります。

これらの苦情が薄れるまでには、数ヶ月かかることもありますが、開発者たちは徐々に厳格なコードレビューの価値を認識し、優れたコードを生成するのにどれほど役立つかを見るようになります。時には、一番声の大きい抗議者が、厳格さによって提供される価値を本当に理解するきっかけが起きたときに、最も強力な支持者になることさえあります。

紛争の解決

もし上記のすべてを守っているにもかかわらず、解決できない開発者との紛争に直面した場合は、 紛争を解決するのに役立つガイドラインと原則については、コードレビューの基準を参照してください。

緊急事態

時には、できるだけ早く全体のコードレビュープロセスを通過する必要がある緊急CLがあります。

緊急事態とは何ですか？

緊急事態のCLは、以下のような小さな変更です：メジャーローンチをロールバックせずに続行できるようにする、本番環境のユーザーに重大な影響を与えるバグを修正する、緊急の法的問題を処理する、重大なセキュリティホールを閉じるなどです。

緊急事態では、コードレビュー全体のスピードに本当に気を使います。ただし、レビュアーのスピードだけでなく、コードの正確性（緊急事態を解決するかどうか）についても、他の何よりも重視すべきです。また、（おそらく明らかですが）このようなレビューは、発生した場合には他のすべてのコードレビューよりも優先されるべきです。

ただし、緊急事態が解決された後は、緊急事態のCLを再度見直し、より詳細なレビューを行う必要があります。

緊急ではないものは何ですか？

明確に述べると、以下の場合は_緊急ではありません_：

• 来週ではなく今週にローンチしたいということ（ただし、パートナーとの合意など、実際の厳しい締切がある場合を除く）。
• 開発者が長い時間をかけて機能を開発し、CLを取得したいと思っている場合。
• レビューアーが現在夜間である別のタイムゾーンにいるか、オフサイトで不在である場合。
• 金曜日の終わりであり、開発者が週末に出発する前にこのCLを取得するのが良いと思われる場合。
• マネージャーが、ソフト（厳しいではない）締切のため、このレビューを完了し、CLを今日中にチェックインする必要があると言っている場合。
• テストの失敗やビルドの破損を引き起こしているCLをロールバックする場合。

などなど。

ハードデッドラインとは何ですか？

ハードデッドラインとは、もしもそれを逃すと何か災害的なことが起こるというものです。例えば：

• 特定の日までにCLを提出することは契約上の義務です。
• 特定の日までに製品をリリースしなければ、市場で完全に失敗します。
• 一部のハードウェアメーカーは年に一度しか新しいハードウェアを出荷しません。彼らにコードを提出する期限を逃すと、提出しようとしているコードの種類によっては、それが災害的な結果をもたらす可能性があります。

リリースを1週間遅らせることは災害的ではありません。重要な会議を逃すことは災害的かもしれませんが、しばしばそうではありません。

ほとんどの締め切りは、ハードデッドラインではなく、ソフトデッドラインです。それらは特定の時間までに機能が完了することを望んでいます。重要ですが、それを達成するためにコードの品質を犠牲にするべきではありません。

リリースサイクルが長い（数週間）場合、次のサイクルの前に機能を追加するためにコードレビューの品質を犠牲にすることは誘惑されるかもしれません。しかし、このパターンは、プロジェクトが圧倒的な技術的負債を蓄積する一般的な方法です。開発者がサイクルの終わり近くに「絶対に入れなければならない」という理由だけで表面的なレビューだけでCLを提出し続ける場合、チームはプロセスを変更して、大規模な機能変更がサイクルの初めに行われ、十分な時間があるようにすべきです。