チケットに対するプルリクエストのレビュー
プルリクエストが完了する前の最後のゲートです。この差分が、アーキテクチャの範囲内でチケットを実現しているかという1つの問いに答え、レビュアーの好みよりも優先される2つの情報源(受入基準と参照アーキテクチャ)に照らして判断します。
このレビューは読み、報告します。編集はしません。見つけた問題を修正するレビューは、何が間違っていたかの証拠を破壊し、誰もパターンを学べなくなります。
3つのフェーズの最後であり、TICKET-ANALYSIS.md と IMPLEMENTATION-PHASE.md に続きます。 WORKFLOW.md は、それらがどのように連携し、インストール方法を示しています。
目次
1. 実際にレビューする内容を特定する
ブランチをリポジトリの実際のベースブランチに対して差分を取ります。それがどのブランチであるかについて推測に基づいてはいけません。すべての変更ファイルをレビューします。「つまらなそう」という理由でスキップしたファイルに、設定変更が潜んでいます。
差分がチケットで言及されていないものに触れている箇所に注意してください。そのリストは §2 と §5 への入力であり、まだ指摘事項ではありません。
2. 受入基準を最初に確認する
受入基準を1つずつ取り上げ、それぞれについてそれを満たすコードとそれを証明するテストを特定します。両方が存在しなければ、満たされていません。
- コードはあってもテストがない基準は、証明されていません。
- テストはあってもコードがない基準は、フレームワークをアサートしているだけのテストです。
- まったく見つからない基準は、最も重要な指摘事項であり、レビュー内のすべてのスタイルに関する指摘よりも優先されます。
次に、逆の方向も確認します。背後に基準がない変更です。一部は正当なもの(変更対象のコード内部の修正、IMPLEMENTATION-PHASE.md §2)です。残りはスコープクリープであり、リリース後に説明するよりも、ここで指摘する方がコストが低くなります。
3. アーキテクチャパス
差分が触れるレイヤーのコンプライアンスチェックリストを実行します。最も価値のある指摘事項は、ビルドが成功しても表示されないものです。
- P2 — 共有カーネルに追加されたドメインに関連するもの:エンティティ、DTO、列挙型、シードデータセット、価格定数、ユーザー向け文字列。
- P3 — コンテキストをまたぐ読み取りまたは書き込み;所有サービスのデータベースにアクセスする別のサービス。
- P4 — マイグレーションなしでのスキーマ変更、またはプロバイダーに依存しないマイグレーション。
- P5 — ソースコード、設定、コメント内のシークレット、キー、接続文字列;環境以外から読み取られる設定。
- P8 — 起動時にフォールバックではなく失敗する新しい依存関係、またはヘルスエンドポイントや起動バナーに含まれていない新しい依存関係。
- P9 / P10 —
Program.csにインライン化された配線;DI に登録されたインターフェースではなく基底クラスとして追加された拡張ポイント。 - P11 — ドメインに未変換で到達する外部形状。
- P15 — トレース、メトリクス、ログがない新しいパス。
SERVICE-API-PATTERNS.md— トラストグループ外のエンドポイント、クリップされていないリスト、不均一なエラーボディ、明示的なタイムアウトのないアウトバウンドコール、再試行された不確定な書き込み。- セキュリティ — 新たに公開されたものはすべて、公開前に
SECURITY-REVIEW.mdパスを実行する必要があります。
プルリクエストが理由付きの決定として記録している逸脱は、指摘事項ではありません。同じ逸脱でも、黙認されているものは指摘事項になります。これが P14 の役割です。
4. テストパス
テスト本体を読みます。テスト数、ファイル数、フォルダ構造は何も証明しません。2026年8月14日に行われた447件のテストスイートの監査では、約45%がプレースホルダーか、実際に何もアサートしないようにガードされており、それらは外部からは見えませんでした(P13)。
- 各新しいテストは、ロジックを保持するレイヤーにありますか?それともブラウザがバリデーターの代わりをしていますか?
- 結果をアサートしていますか?それともボタンがクリックされたことをアサートしていますか?
- 差分が触れる既存のパスごとに回帰テストがありますか?それとも新しいブランチのテストだけですか?
- テストごとの基準(独立している、
data-testidを使用、スリープなし、本番資格情報なし)を満たしていますか?(TESTING-STRATEGY.md§6) - 新しいテストエントリポイントは CI コンテキスト(§9)で実行されますか?
5. 重大度と判定
すべての指摘事項をランク付けし、説明のしやすさではなく影響に基づいてランク付けします。
| レベル | 意味 |
|---|---|
| ブロッキング | 受入基準が満たされていない、または証明されていない;記録された決定なしに原則が破られている;シークレットが露出している;テストが弱められた、または削除されてグリーンステータスになった |
| 修正すべき | 正しいがパターンから外れている:間違ったテストレイヤー、回帰カバレッジの欠落、記録されていない前提、変更によって不正確になったドキュメント |
| 検討 | 本当にオプション。このセクションは短く保つこと — 長くなると最初の2つが埋もれてしまう |
図のゲートが必要とする判定を明確に記述して締めくくります。このプルリクエストはチケットを実現しており、アーキテクチャの範囲内ですか? 3つの回答のいずれか:検証済み;リストされた非ブロッキングの指摘事項とともに検証済み;またはブロッキングの指摘事項が特定され、未検証。「良さそう、いくつかコメントあり」は判定ではありません。
6. 失敗モード
| 症状 | 原因 |
|---|---|
| レビューが要件を満たしていない差分を承認する | コードからではなく受入基準からレビューを開始しなかった |
| レビューがスタイルに関する意見のリストになる | アーキテクチャパスとテストパスが、ビルドがグリーンだったためにスキップされた — まさにそれらがチェックするもの |
| グリーンスイートが証拠として扱われる | テスト本体が読まれず、数と構造が信頼された |
| すべての指摘事項が同じ重みで報告される | 重大度ランク付けがないため、ブロッキングの指摘事項が命名の些細な指摘の下に埋もれる |
| 同じ違反がプルリクエスト間で繰り返される | 指摘事項がレビュアーによって報告されるのではなく修正されたため、パターンが学習されなかった |
| 作成者がすでに正当化していた逸脱が報告される | プルリクエストの記録された決定がレビュー前に読まれなかった |
7. チェックリスト
- リポジトリの実際のベースブランチに対して差分を取った;すべての変更ファイルを読んだ
- 各受入基準を、それを満たすコードとそれを証明するテストにマッピングした
- 背後に基準がない変更を特定し分類した
- 差分が触れるすべてのレイヤーについてコンプライアンスチェックリストを実行した
- 記録された決定を最初に読んだため、正当化された逸脱を指摘事項として報告しなかった
- 新たに公開されたものすべてにセキュリティレビューパスを与えた
- テスト本体を数ではなく読んだ;レイヤー、結果のアサーション、回帰カバレッジ、テストごとの基準をチェックした
- 指摘事項をブロッキング/修正すべき/検討にランク付けした
- チケットに対する明示的な判定を下した
- レビュー中に何も編集しなかった
docs/delivery/PR-REVIEW.md から scripts/build-marketplace.mjs によって生成されました。このファイルを編集しないでください。ソースドキュメント、または catalog/marketplace.catalog.json のエントリを変更し、ジェネレーターを再実行してください。