GitHubのPRベースでコードレビューをする現場に入ったので、改めてコードレビューについて自分用にまとめておく
レビューによるコードの改善はレビューアとレビューイの共同作業。
コードレビューにおいて最重要なのは、敬意と礼儀。お互いを尊重することが大切。
レビューは、レビューイを助けるためのプロセス。
レビューの重要な目的はコードベースの可読性を高く保ち、一定の生産性を維持すること。
レビューイの助けになることと、生産性を維持することのバランスをとる上で重要な４つのポイント。
- レビュー依頼を放置しない
- 問題のあるプルリクエストを拒否する
- 締切を意識しすぎない
- 「提案」以外のコメントを考慮する
レビュー依頼を放置しない
- レビュー依頼を受けた時、入るまでに最初の返信をするかという期限を、チームのルールとして決めておく
- レビューをする期限ではなく、あくまでも返信の期限にすると良い
- 優先度の高いタスクを大量に抱えていて、すぐにレビューできないような時は、レビューの返信期限までに自分がどういう状態なのかを伝えることが大切
- 実際にはレビューできないのに「レビューできる」と返信するのは避けるべき
問題のあるプルリクエストを拒否する
- プルリクエストが大きすぎる場合や基本的な設計が間違っている場合など、大きな問題のあるプルリクエストは一旦クローズしてもらい、新たに作り直してもらうことも必要
- その場合は、どのように分割するかや、構造について焦点を絞ってレビューするなど、必ずフォローアップをすること
締切を意識しすぎない
- 「忙しい」「締め切りが近い」などの理由でレビューの品質を下げるべきではない
- レビューのプロセスを急ぐ必要がある場合はその理由を説明してもらう
- 場合によっては、仕様の変更で対処できたり、機能実装を次回のリリースに延期できる可能性もある
- レビューイが「絶対に期限までにマージしなくてはならない」という先入観にとらわれているケースもあるので、より広い視野でレビューを
- どうしても急がなければならない場合は必要に応じて以下を実施する
- 後でどのようにコードを改善するか、大まかな方針の合意をとっておく
- その場しのぎのコードに対するコメントとテストを残しておく
- イシュー管理システム上で「将来のコード改善」のチケットを作成しておく
「提案」以外のコメントを考慮する
- レビューコメントで改善の提案を行う場合、具体的かつ明確な内容を書く
- ただし、コメントで詳細を詰めようとすると多くの時間をかける必要があるためレビューアとレビューイの負荷のバランスが崩れてしまうので注意
- どの程度具体的な提案をするかの段階の例
- 問題点がないか確認を促す
- 問題点を指摘する
- 問題点とその再現方法や理由を提示する
- 問題を解消するための手法をいくつか提示する
- 問題を解消するための最適な手法を提示する
- 問題を解消するためのコードを定案する
- 問題点のハッケイン以前にコードで不明な点がある場合はレビューイに質問する方が効率的
- 指摘内容を伝えるにしても、質問をするにしても必ずしもレビューコメントを使う必要なく、対面の議論やチャットなども選択肢に入れる
- その場合は議論した内容や結論を要約してレビューコメントに残しておく
また、GoogleのChromiumプロジェクトのレビュー指針、「尊敬に満ちたコードレビュー」も参考になる
コメント（指摘）の内容
- プログラミング原則
- 命名
- コメント
- 状態
- 関数
- 依存関係
- コーディングスタイル・コーディング規約・言語やプラットフォーム固有のイディオム
- テストコードや動作の確認手段
- プルリクエストやコミットの大きさ・構造
- タスクそのものの目的・範囲
- コードの複雑性と達成したいことのバランス
- バグやセキュリティ上の欠陥
- パフォーマンスやフットプリントの変化
ただし、上記の中で静的解析ツールなどによって自動で指摘できるものはできるだけツールに任せること
また、「気が付かないうちにコードベースの可読性が下がっていた」という状況を避けるため、変更があったコードだけでなく、周辺のコードや依存関係のあるコードも確認する必要がある
コードレビューは設計的な妥当性に重点を置いてレビューする
参考書籍
- 読みやすいコードのガイドライン 石川宗寿 著
- 良いコード／悪いコードで学ぶ設計入門 仙塲大也 著