コードレビュー
ユーザーが指定した変更(差分、ブランチ、PR、またはコミットされていない作業ツリー)をレビューします。指定がない場合は、コミットされていない変更、または現在のブランチをベースブランチと比較してレビューします。
レビューの方法
- まず意図を理解する。 PRの説明、コミットメッセージ、関連するIssueを読みます。変更は、それが達成しようとしている目的に対してのみ誤っている可能性があります。
- 差分全体を読み、次に周辺のコードを読む。 変更された関数と呼び出し元を開きます。実際のバグのほとんどは、新しいコードと既存のコードの境界に存在します。
- 以下の順序でチェックする:
- 正しさ: 論理エラー、オフバイワン、誤った条件、未処理の
null/空/エラーケース、競合状態、リソースリーク、壊れたエラー伝播。 - 振る舞いの変更: マイグレーションや通知なしに変更された公開API、ファイル形式、デフォルト値、データベーススキーマ。
- セキュリティ: 信頼できない入力がクエリ、シェル、ファイルパス、HTML、またはデシリアライザに到達する場合、コードやログ内のシークレット、不足している認可チェック。
- テスト: 変更を元に戻すとテストが失敗するか?エッジケースはカバーされているか?
- 可読性: 誤解を招く名前、2つのことを行う関数、コードと矛盾するコメント、デッドコード。
- 正しさ: 論理エラー、オフバイワン、誤った条件、未処理の
- 報告する前に検証する。 疑わしいバグごとに、それを引き起こす具体的な入力をトレースします。具体化できないものは削除するか、質問としてラベル付けします。
報告方法
- 先頭に1行の判定を記載する: マージ準備完了、修正後マージ、または再作業が必要。
- 最も深刻なものから順に記載する。それぞれについて:
ファイル:行、問題内容、具体的な失敗シナリオ、および修正提案。 - 必ず修正するもの と 提案 および 質問 を分ける。
- フォーマッターやリンターが検出するようなスタイルの微調整でレビューを水増ししない。また、差分が何を行うかを再述しない。
- 注目すべき良い点(明確なテスト、注意深いマイグレーション)があれば、1行で簡潔に述べる。