なぜやるか

方法

• Googleが用意しているコードレビューガイドラインを元にコードレビューに関しての理解を深める
• https://google.github.io/eng-practices/review/
• https://fujiharuka.github.io/google-eng-practices-ja/ja/review/developer/
• 日本語（非公式）
• 基本的にはこちら読んでまとめたものを共有する

• その後、弊社にとってやりやすいやり方を議論する

勉強会の背景

弊社のコードレビューの課題

• スピード
• timezoneが違う人が多くレビューが遅い
• 個々のメンバーが自身の実装タスク重視でレビューは時間ができたらやるぐらいの温度感になってそう
• それが故にレビュワーの負担を考慮したPRの粒度を考えられていない

• 内容
• コードレビューで何を見るべきかわかっていない
• レビューの出し方/コメントの伝え方
• なぜそのコメントをしたのかわかりにくいことがある

• やりかた
• レビュワーが動作確認までするべきかなどどこまでやるかはレビュワーによって異なっていそうなので、共通認識を作りたい。

今回何をするか

内容

Introduction

• コードレビューとは、コードの筆者とは別の人がコードを検証するプロセスです

• コードレビューは、コードとプロダクトの品質を維持するのに行われます。

どのようにコードレビューを行うのか（開発者、レビュアー）

コードレビューの基準

• コードの健全性をあげていくのが目標です。

• 妥協点のバランスを取っていく必要がある。

一番の目的：システム全体のコードの健全性を向上させること

• 「よりよくする」コードを提出しなければ、コードの健全性は上がらない。

• もしレビュアーが変更を入れるのを難しくしたら、今後改良を行う意欲がなくなる

• レビュアーの義務はコードの健全性が低下しない品質であることを確認すること。
• 時間制約のせいで短縮する場合、ちょっとずつコードの健全性が低下しがち

• レビュアーはオーナーシップと責任感をもってやらねばならない
• 一貫性、メンテナンス性等がコードに維持されるのを求められる

• PRが完璧でなくても、作業中のシステム全体のコードの健全性を確実に向上させるのであれば、承認するのを優先すべき
• ただし望んでいない機能が追加されていたりする場合は承認を拒否できる

• 必ずしも「完璧」でなくても、「よりよい」コードであれば良い
• 「細部の完璧」を求めて、時間がかかるのは本末転倒である
• あまり重要でない修正点の場合は’Nit:’等前置きをおこない、「無視しても良い改善点」として伝えるようにしましょう。

• システム全体の健全性を確実に悪化させるPR を正当化しません。緊急時のみです。
• 緊急時に関しては、まず緊急事態を解決できているかを最優先し、解決後に徹底的なレビューを行うべきです

メンタリング

• 知識を共有することはコードの健全性を向上させる方法の一部

• だが純粋に教育的なもので、コード全体の健全性の向上にそこまで寄与しない場合は、’Nit: ’等の前置きをして、解決が必須ではないことを示すべき

原則

• 技術的事実、データ＞個人の嗜好、意見

• スタイルに関しては、スタイルガイドを絶対とする。
• 逆にガイドにない部分は作者のものを受け入れる

• ソフトウェア設計は基本的な原則に基づくべき

• 他のルールが適用されない場合、現在のコードベースに合わせることを依頼できる。

• レビュアー、開発者でこのレビューガイドを元にして合意形成を図っていくべき。
• 合意形成が困難な場合、コメントのみでなく対面の会議等で解決を図るべき
• その場合コメントに経緯と決定事項を記載しておくこと
• それでも合意できない場合はより広いチームでの議論、他のメンテナ等に判断を仰ぐようにする

開発者側（PRを出す方）

良いChangeLogの記述方法（開発者側）

• 最初の行で、何をするかの要約を書く（バージョン管理の履歴サマリに表示される）
• わかりやすく、実用的であること
• 英語の場合は命令形で記載する

• 2行目から数行空白をあける

• 本文は詳細な情報提供
• 変更リストを全体的に理解するために必要な補足情報を書く
• 解決しようとしている問題の説明
• なぜこのアプローチなのか
• アプローチに欠点が想定されるならそれも記載する
• バグ数、ベンチマーク、設計文書へのリンク、背景情報等
• 外部リソースへのリンクを含める場合は、将来の読者が閲覧できない可能性にも配慮すること。できれば、PR内で理解が完結するようだと好ましい

不適切な記述

• 「何を」「何故」「どうやって」変更したがわからない記述
• bugfix
• build修正
• フェーズ1
• 変なURLを消す

小さくてシンプルなPRにしましょう

• レビュースピードが早い

• 徹底的なレビューができる

• 変更箇所が少ない＝バグ発生が少ない

• PR却下時の無駄な作業が減る

• マージがしやすい

• 小さい方が設計がしやすい

• レビューでブロックされることが少ない

• ロールバックが簡単

• 「PRが大きすぎる」という理由で、レビュアーは変更を完全に拒否する裁量がある。

小さいPRの最低要件

• 自己完結していること

• テストコードをすべて含む

• 現状の変更でレビュアーが理解する必要のあることはすべてPRに書かれていること

• 理解しづらいほどちいさくはしない

• 「大きすぎる」の厳密なルールはない

レビュアーコメントへの対応

• 個人攻撃に受け取らない

• レビュアーのフラストレーションがコメントで現れてることがあるが、その中から「伝えようとしている建設的なことはなにか」を自答して行う

• コメントに怒りのままに反応するのはエチケット違反！

• レビューはレビュアーとの共同作業。