コードレビューでは、細かな書式を読む前に、何を変えるための変更で、何が変わってはいけないかを確認すると、指摘の優先順位を付けやすくなります。コードが整って見えても、目的と違う動作になっていれば修正が必要です。
この記事では、業務アプリやクラウド運用コードを確認するエンジニア向けに、レビューの観点とコメントの返し方を整理します。すべての言語に共通する合否チェックリストではなく、変更内容に応じて使う確認の順番です。
変更目的と対象範囲を読む
最初に、依頼や不具合の説明と、実際の差分が対応しているかを確認します。期待する挙動、再現条件、変更対象、確認結果が分からなければ、コードだけから意図を推定し続けず、作者に不足情報を求めます。
たとえば設定ファイルの更新なら、どの環境の何を変えるかが必要です。APIの改修なら、入力・出力・失敗時の扱いで何が変わるかを確認します。意図に関係しない大きな整形や別機能の修正が混ざっていないかも見ます。
Googleのレビュー資料も、設計、機能、複雑性、テストなどを確認対象に挙げています。書式だけでなく、変更がシステム全体にどう組み込まれるかを見る考え方です。Googleのコードレビュー観点
観点を、変更で壊れ得ることへ結び付ける
| 観点 | 確認する問い |
|---|---|
| 目的との一致 | 要求された動作を実現し、関係しない挙動を変えていないか |
| 境界条件 | 空、0件、上限、欠損などでどう動くか |
| 失敗時 | 途中で失敗した状態と、再実行時の扱いは明確か |
| データと権限 | 扱う情報や操作対象が意図した範囲か |
| テスト | 不具合が戻ったときに失敗する確認になっているか |
| 保守性 | 後から変更する人が、責務と前提を追えるか |
すべての変更に同じ数の指摘を出す必要はありません。たとえば表示文言だけの変更と、データを更新する処理では、重点的に見る箇所が異なります。影響に応じて、必要な専門性を持つ担当へ確認を分担します。
仮例:対象一覧が空のときの処理
以下はレビューの考え方を示す仮例です。実際に発見した不具合の報告ではありません。
ある処理に、対象IDの一覧を渡すとします。通常の入力では期待どおり動いても、一覧が空の場合に「対象なし」として終了するのか、「条件なし」として全件を扱うのかで、結果は大きく異なります。
レビューでは、空になる経路があるか、呼び出し先が空をどう解釈するか、期待する結果を確認します。呼び出し元だけ見て判断せず、必要な範囲で関数やAPIの契約へ戻ります。
テストについても、正常な一覧の例だけでなく、空の入力に対して何を期待しているかを確認します。テストが通ったという報告だけで、境界条件を確認済みとは扱いません。
コメントは、場所・影響・期待する挙動を揃える
「ここが分かりにくい」「危なそう」だけでは、作者が何を変えるべきか判断しにくくなります。問題だと考える箇所、どの条件で何が起きるか、どうなってほしいかを組み合わせます。
仮例なら、「対象一覧が空のとき、呼び出し先では全件処理として扱われます。対象を限定する変更目的と異なるため、空なら処理しない条件と、その確認を追加してください」のように書けます。ただし、呼び出し先の挙動が未確認なら、断定せず確認質問として返します。
実装方法を一つに固定する必要がない場合は、期待する挙動と確認条件を示します。代案の余地を残すことで、目的に合う小さな修正を検討できます。
必須修正と任意改善を区別する
不具合、要求違反、重大な保守上の問題と、好みの書き方は分けます。Googleの資料でも、個人的なスタイルの好みだけで変更を止めない考え方が示されています。
チームの規約があるならそれを参照し、任意の提案には任意と分かる表現を付けます。指摘が多い場合は、公開や統合を止める理由から先に伝え、細かな改善だけで全面的な書き直しを反復しないようにします。
修正後は、指摘と影響範囲を再確認する
修正された行だけでなく、その変更によって関連する条件が変わっていないかを確認します。一方、変わっていない部分の確認を最初から繰り返す必要があるかは、版と検証結果を見て判断します。
レビューを終えるときは、確認した範囲と、別担当に委ねた範囲を明確にします。コードレビューだけで本番の動作や運用のすべてを保証するものではないため、必要な試験や公開前の確認と組み合わせてください。
運用作業への引き渡しが必要なら、運用手順書の作り方も参照できます。コードの変更目的と、運用担当が確認する結果を揃えておくことが、引き継ぎ時の判断材料になります。