Google's Engineering Practices documentation は読んでいてためになったので気になった部分をメモ。
Writing good CL descriptions
- 何故変えたのかと何を変えたのか
First Line
- what & 命令文 (Delete xxx instead of Deleting xxx)
Body is Informative
- why
- 欠点があるなら書く
- 背景 (bug number, benchmark, design doc, etc.)
Bad CL Descriptions
Good CL Descriptions
Review the description before submitting the CL
Small CLs
Why Write Small CLs?
- レビュースピードが早まる
- 細かく見れる
- バグになりにくい
- reject された時に無駄が少ない
- マージしやすい
- 巨大な変更よりはデザインしやすい（いまいちわかってない）
- 自己完結してる変更にしとけばそれがレビュー中でも別の変更をコーディングできるのでブロックされにくい
- ロールバックしやすい
What is Small?
- 自己完結してる一つの変更
- just one thing
- 将来的な追加を考えずに CL だけ見れば分かる
- 入れても動かせる
- 理解するのが難しいほどに小さすぎない （API 追加するなら、それを使うものも)
- （決まりはないけど）100 行なら大抵は妥当、1 ファイル 200 行は分かるけど 50 ファイル 200 行はでかい
- 自分が書いたものに関しては背景など精通してるため、自分が思っている許容可能なサイズは実際の reviwer が許容可能なサイズよりも大きくなりがち （試しに小さく送っても文句言われることは稀だろう）
When are Large CLs Okay?
- ファイル削除
- 機械的な処理
Separate Out Refactorings
- リファクタリングを機能追加や bugfix に混ぜない
Keep related test code in the same CL
- テストコードを変更と分けない
Don’t Break the Build
- 動き続ける CL にする
Can’t Make it Small Enough
- すごい稀なはず
- リファクタリングを先にすることを考える
- 極めて稀なはずだがある場合は出す方も見る方も気合い
How to handle reviewer comments
Don’t Take it Personally
- コードに対する批評を個人を攻撃していると捉えて怒らない
Fix the Code
- コードが理解できないと言われたらコードを整理してコメントを入れる (たぶん reviwer が分からなかったら将来見る人もわからない、その時に review tool のコメントは見れない）
Think for Yourself
Resolving Conflicts