あるときコードレビューするときにどういうところ見てるんですか？ と訊かれてたしかに自分でもあまり言語化したことはなかったな、と気づいたので簡単に書いておく。
変更意図が要求に沿っているか
- そもそも実現しようとしていることが、ユーザやプロダクトオーナーの要求に沿っているか。モデリングや実装のコンテキストを自分でも把握しておく。
- 関連する別の変更やイシューなど、自分が知っていて相手が知らない有意義な情報があったらコメントする。
モデリングが妥当か
- モデルによって意図が表現できているか。仕事が適切な粒度で明確に切り分けられているか。意図のない共通化がなされていないか。
- わかりやすい名前がつけられているか。ここが混乱していると何かがよくないサイン。既存のコードがすでに……ということもある。そういう場合は改善できそうな道筋について議論できるとベター。
- 仕事にあったインタフェースになっているか。テストを書いたときに素直なコードになっていると望ましい。例外の取り扱いや、アウトプット先を変えたときに無理なく使えそうか、とかも考える。
実装が妥当か
- プロダクションでの性能が意識されているか。逆に無意味に性能を追ってコードが読みづらくなっていないか。
- アルゴリズムに誤りがないか。とくに外部・外界とのやりとりを伴う場合、エッジケースがテストされているか。
- 異常が発生したときにデバッグ・調査しやすいか。エラー（ログ）に必要な情報が含まれているか。
- 既存のコードが今回の意図に合わせて変更されているか。
読み手の負荷が高くないか
- コメントが書かれているか。ドキュメントやドキュメントへのリンクがあるか。
- 読みづらいコードになっていないか。やむなくそうなっている場合はコメントがあるか。Pull Request のコメントで補足される場合があるけど、そういうときはコメントに書いてもらうようにする。
- 議論の最中で出てきた未来の仕事が TODO とか FIXME となってコメントに残されているか。
- この言語では/チームでは/プロダクトではしない書き方がないか。このへんは完全に知識の問題で、逆に教えてもらうチャンスでもある。機械がやってくれてもいいところ。
ざっと書き出したものなのでこれが全てではないと思います。
意図を理解したら、変更を順に見ていき、必要に応じて実行フローの前後を確認して、違和感があったり理解できないところにコメントをつけていくような流れ。
また、指摘や質問するだけではなく「ここはこう読み取りました」というコメントを残すこともある。これは認識を揃えるため。コードレビュー初心者で何をしたらいいかわからない、という人はこの「俺はこう思ったっす」を書いていくところから始めたらよいと思う。
