コードレビュー観点
本ドキュメントでは、コードレビューを行う際のガイドラインとチェックリストをまとめます。
PRの記載内容
タイトルと内容に関して、以下の観点で記載されているか確認します。
タイトル
- タイトルだけで何の対応をしたかわかるようになっているか。
- 対応したチケットの番号や機能のIDが記載されているか。
- 識別子だけでなく、テキストでも対応内容が記載されているか。
内容
- 対応内容:
- どのような対応をしたか。
- 対応するに至る経緯は何であるか、など。
- 修正箇所:
- 具体的にどのような修正をしたか。
- どのように考えて修正したか、など。
- 確認に必要な追加情報:
- パッケージを追加した場合には、その詳細がわかるリンク。
- その他、前提知識として必要な情報を記載しているか。
- テスト観点:
- どのようなテストを行ったか。
- エビデンス（スクリーンショットやログなど）が添付されているか。
- 関連ページ:
- 参考にしたドキュメントやPRなどが記載されているか。
コメントのプレフィックス
レビューコメントの意図を明確にするため、以下のプレフィックスを活用してください。
- must: 対応が必須の指摘。
- want: 可能であれば対応してほしい提案。
- nits: 些細な指摘（修正の可否は問わない）。
- Q: 質問や意図の確認。
コードのチェック観点
以下の項目を重点的にチェックします。
INとOUTの確認
実装したコードのインプットとアウトプットが、設計（I/Fなど）と一致していることを確認します。
- 画面・UIの場合: 呼び出しパラメータの妥当性の確認。
- APIの場合: リクエストとレスポンスの妥当性の確認。
ロジックの確認
実装内容が正しいかを以下の観点で確認します。
- 取得SQLの条件設定が合っているか。
- 追加・更新・削除のパラメータが合っているか。
- 処理内の条件分岐に問題がないか。
- エラーハンドリングが適切に行われているか。
テストの網羅性
テストが十分に実施されているかを以下の観点で確認します。
- テストコードがある場合:
- 記述方法が正しいか。
- 必要なパターンが網羅されているか。
- テストの実行速度に悪影響を与えていないか。
- テストコードがない場合:
- 代わりにどのようなテストを行ったか記載があるか。
- 実行結果（エビデンス）が記載されているか。
- 十分なパターンを網羅しているか。
記述方法の統一
プロジェクト全体の標準に従っているかを確認します。
- 全体の実装フローが統一されているか（例: controller → transaction → service → repository）。
- 命名規則に統一感があるか。
- ファイルの分割粒度に統一感があるか。
命名規則
意図が伝わる命名になっているかを確認します。
- 実体系（クラス・フィールドなど）:
- 何の実体かわかる名前になっているか。
- フィールドに格納される値が推測できるか。
- 動作系（関数・メソッドなど）:
- 動詞から始まっているか。
- 内部処理を簡潔に表せているか。
- 戻り値の内容が名前から推測できるか。
クラス・関数の分割粒度
関数の中の処理は極力シンプルに保つようにします。
分割が不十分だと可読性が低下し、再利用時の認識齟齬によるバグの原因となります。
関数名と処理内容が一致しない場合は、粒度に問題がある可能性があるため、さらなる分割を検討してください。
コメントの適切さ
命名だけで意図が伝わることが理想ですが、複雑なロジックや特殊な処理にはコメントを記述することを推奨します。
- 複雑な仕様が組み込まれる場合には、背景や意図をコメントで補足してください。
- ※コメントの言語はプロジェクトの方針に従います。
禁止事項・アンチパターン
以下の項目に該当しないか確認します。
- マジックナンバーの直接入力:
- 意味を持つ数値や文字列は定数として定義し、名前を付けて使用してください。
- 関数・クラスの肥大化:
- 1つの関数が100行を超える、あるいは1つのクラスに責任が多すぎる（単一責任原則に反する）場合は、分割を検討してください。
- デバッグコードの混入:
console.log や一時的なコメントアウト、未使用の変数が残っていないことを確認してください。
- 感情的なレビュー:
- レビューは「コード」に対して行うものであり、「人格」を否定するような表現は避けてください。
- 攻撃的な言葉遣いをせず、建設的なフィードバックを心がけてください。
関連ドキュメント: