コードレビューをやる場合はこれを見ながらやるとよさそう
現場のコードレビュー時に以下のようなトレードオフが存在する
- どんな変更に対してもレビュアーがいちいち難色を示して変更を取り入れなかったら開発者の改善意欲が削がれる
- 一方で、レビュアーはコードベースの健康状態が悪化しないようにきちんと確認が必要である
この問題に対して、何かしらのガイドラインがあった方が良いということで、このガイドラインを作られたとのこと
コードレビューの全ガイドラインで最上位の原則は以下
一般に、変更が完璧でなくても、その変更がシステムのコードの全体的な健康状態を改善すると確実にわかれば、レビュアーは変更を積極的に承認すべきである
「完璧さ」は求めておらず、「継続的な改善」を原則としているのが注目ポイント
コメントはためらわず残すべきであるが、重要ではないものには「Nit: 」（あら探しや細かい指摘という意味）のようなプレフィックスを付けて無視してもらっても構わない作成者に知らせるべきだと述べられている
レビューする上でのその他の原則
- 自分の意見や好みを捨て、技術的な事実とデータでレビューする
- スタイルガイドが絶対的な権威であるが、ない場合は前例に従い、前例もなければ作者のスタイルを受け入れるべき
- ソフトウェア設計において複数の方法が同等に有効であると証明できる場合は作者の選択を受け入れるが、証明できない場合はの標準的な原則に従うべき
- 上記以外ではコードベースを悪化させない限りは一貫性を維持するように求めて良い
意見が対立した場合
- 対面でのミーティングや電話会議
- 第三者を巻き込む
コードレビューの観点
コードレビューの観点も記載されている
コードレビューの観点 | google-eng-practices-ja
要約を引用する
コードレビューをする際には、次のことを確認する
- コードがうまく設計されている
- 機能性がコードのユーザーにとって適切である
- UI の変更がある場合、よく考えられていて見た目も適切である
- 並行処理がある場合、安全に行われている
- コードが必要以上に複雑でない
- 開発者は将来必要になるかもしれないものではなく、現在必要だとわかっているものを実装している
- コードには適切なユニットテストがある
- テストがうまく設計されている
- 開発者はあらゆるものに明確な名前を使った
- コメントは明確で有意義なもので、「何」ではなく「なぜ」を説明している
- コードは適切にドキュメント化されている
- コードはスタイルガイドに準拠している
レビューを依頼されたコードを一行ずつレビューすること、コンテキストを確認すること、コードの健康状態を改善しているかを見極めること、開発者が良いことをしたらそれを褒めることを忘れない
複数ファイルあって大変な時
複数ファイルあって大変な時の手順の要約
レビューで CL を閲覧する | google-eng-practices-ja
- ステップ 1: 変更を広く眺める
- ディスクリプションなるものを書いてもらう
- 却下の代わりに丁寧に提案する
- ステップ 2: 変更の主要部分を調べる
- 変更の「メイン」の部分になっているファイルを見つける
- 変更があまりに巨大でどの部分が主要部分なのか判別つかない場合は開発者にどこを最初に見るべきか質問するか、変更を小さくしてもらう
- ステップ 3: 変更箇所の残りを適切な順序で見る
- 主要なファイルを確認し終えたら、普通はコードレビューツールが表示してくれる順序で各ファイルを調べると楽
- 主要なコードを読む前にテストをまず読むのが効果的な場合もある
コードレビューのスピード
コードレビューのスピードについて
コードレビューのスピード | google-eng-practices-ja
- コードレビューの依頼が来たらすぐに着手（最長の時間は一営業日）
- コードを書くような集中的に取り組むべきタスクの最中には、自分のタスクを中断してコードレビューしてはいけない
- 時間が取れなそうであれば全体のレビューが完了をする前に、部分的にでも素早いリアクションをとる（「このコードは私達の基準を満たしている」という意味だと言えるくらいに素早くレビューする）
- 以下の場合は指摘を残しつつ承認しても良いが、どちらの意図なのかをはっきりさせる
- 開発者がレビュアーの残したコメントに後で着実に取り組んでくれるとレビュアーが信頼できるとき
- 先送りにした変更がさほど重要でなく、開発者本人が必ずしも行う必要のないとき
- コードレビューの基準に妥協しない、スピードを上げてもコードの品質の改善に妥協しない
コメントの書き方
レビューコメントの書き方 | google-eng-practices-ja
上のページを要約すると
- コメントは丁重に
- 理由を説明する
- 問題の指摘に加えて明確な方向性を示すことと、開発者本人に決定を委ねることをバランス良く行う
- 複雑なコードを見つけたらそれを説明してもらうだけで終わらせず、コードをシンプルにしてもらうとかコードにコメントを追加するよう開発者に勧める
取り下げるか否か迷うとき
- 開発者の提案が受け入れられない時、開発者のほうが正しいのではないかということを最初に少し考慮しておく
- その指摘によってさらに作業が発生するものの、それに見合うだけコードの品質が改善すると見込まれれば、粘り強く変更を勧めるべき
- 指摘によって開発者が傷つくのではないかと思う人もいるが、もし傷ついてもそれは一時的なことで、時が経つとコードの品質改善に感謝する
- 開発者が仕事を完了させたいがために「後でやる」と言って指摘取り下げを要求してくることがあるが、大体はだんだんと忘れ去られるため、「今片付ける」ように促す
- 問題を顕在化しているのに開発者がすぐにその問題に取り組めないときには、問題解決のためにバグを整理保存し、忘れないように自身をアサインし、TODOコメントを残す
