Google's Code Review Guidelines を読んでアクションに落とす
Summary
Google's Code Review Guidelines を読み、具体的にどうアクションするかを念頭に置くため、アクションリストだけをまとめました。
- Google Engineering Practices Documentation | eng-practices
- 2019/9/6頃に公開されました。
- Google エンジニアリング・プラクティス ドキュメント | eng-practices
- 2019/9/16頃には全ての翻訳が完了しています。
- 非公式の日本語翻訳。今回まとめる際に大変参考になりました。
レビュアー向けのガイド
The Standard of Code Review 基準
- プルリクのコードが完璧でなくても、確実に改善するのであれば、積極的に承認する。
- 重要でない指摘は、”Nit:” をつけることで、変更するか否かは作者に委ねることを伝える。
- コードレビュー上のコメントでのやりとりが難航しそうなとき、対面のミーティングやビデオ会議を行う。
- そのとき、他のレビューワーのために、議論の結果はコメントとして記録する。
- 合意を得られないからといって、プルリクを放置しない。
What to Look For In a Code Review 何を見るか
- ユーザーへの影響を理解するにはコードだけでは分からないこともあるので、動作確認を行う。
- 難しい場合は、作者にデモをお願いする。
- オーバーエンジニアリングに警戒する。
- 適切なテストを求める。
- コード中のコメントがWhyであるか、Whatになっていないかを確認する。コメントが必要なものかを確認する。
- 個人的なスタイルの好みに基づくものは“Nit:”とする。
- 関連するドキュメントのアップデートを依頼する。
- すべての行を読み、自分で理解する。
- コードを読むのが難しい場合は、明快にするよう作者に伝える。
- レビュアーとして自分がふさわしくないと感じるのなら、他の人をレビュアーにアサインする。
- コードの変更で良いところがあったら、それを作者に伝える。
Navigating a CL in Review どう進めるか
- ステップ1: 説明をよく読む
- プルリクの説明はよく読む。
- 変更が行われるべきでない場合は、すみやかに、理由・代替案を丁寧に説明する。
- 望ましくない変更が多く発生する場合は、コードレビューの前の開発プロセスを見直す。
- ステップ2: メインとなる部分を見つけ、設計を確認する
- まず、メインとなる部分を探す。
- コードの変更が大きい場合は、分割を依頼する。
- 設計に問題がある場合は、すみやかにコメントを送る。
- ステップ3: 他の部分を読む
Speed of Code Reviews スピード
- 集中しているときでなければ、レビューがきたらすぐに行う。レビューで自分の集中を妨げない。
- コーディングが完了したとき、昼食の後、キッチンから戻ってきたときにレビューを行う。
- 1営業日中までにレスポンスする。
- レビューは翌朝の最初に行う。
- 忙しい時は、いつできるかをレスポンスする。他のレビュアーを提案する。最初に大雑把なコメントをする。
- 未解決なコメントがあったとしても、細かい変更であったり、後で変更されるだろうと言う場合は、承認する。
How to Write Code Review Comments コメントの書き方
- 礼儀、あいまいではなくはっきりとした態度、尊敬する気持ちをもつ
- 開発者に対してコメントをしない。コードについてコメントをする。
- 「なぜか」を説明する。
- 問題を指摘することと、具体的なコードを提供することの間で適切なバランスを取る
Handling Pushback in Code Reviews
- 対立がおきたとき:
- 立ち止まり、開発者の意見が正しいのではと考える。
- 理解しようとする姿勢が伝わるようにする。
- なぜ、提案が正しいと思うのかを、丁寧に説明する。
- 後でやる、が発生したとき:
- 「後でやる」が実行されることは少ないので、今すぐやるべきと主張する
- すぐできないときは、チケットを切る
- レビュー基準のブレにより不満が発生したとき:
- 恐らくその不満は、レビューのスピードをあげることで解消できるので、それを検討する
プルリクの作成者向け
Writing Good CL Descriptions
- 1行目:短い要約。コードの変更によって具体的に何がなされたのか。(英語の場合は、命令形を使う)
- 残りの本文は、十分な説明を書く。何を解決したか。なぜそれが良い解決方法であるのか。欠点は何か。なぜ変更したか。
- プルリクを送信する前に、見直す。
Small CLs
- コードの変更は、1つのことだけを扱う。
- 100行は妥当、1000行は大きすぎる。1ファイルの200行は問題ないかもしれないが、50ファイルにわたる200行は大きすぎる。
- 分割した場合は、他のプルリクがあることを伝える。
- リファクタリングは分離する。
- 関連するテストコードは分離しない。
- 大きなコード変更をする前に、先にリファクタリングのみ行う。
- コードの変更が大きくなった時は、レビュアーに了承を得る。
How to Handle Reviewer Comments
- コメントに怒りで返事をしてはいけない。
- レビュアーのコメントが建設的でないときは、対面かビデオ会議する。
- レビュアーがコードを理解できなかった場合、コードを適切に変更するか、コードにコメントを残す。
- コメントが来たとき、反射的に考えるのではなく、立ち止まり、そのコメントは正しいか?をよく考える。
- 対立したら:
- コードレビューの基準を読む。