変更レビュー
このスキルは画面ではなく、変更をレビューします。変更のスコープを解決し、変更されたファイルが影響を与えるサーフェスに展開し、差分の両面を読み、すべての発見項目を分類します。
スコープはこのスキルが担当します。ドメインルールは better-* スキルに属します。重大度、統合、カバレッジ、上限、判定は better-interface に属し、このスキルはレビューを委ねます。
正当性、テスト、セキュリティ、パフォーマンスはプロジェクトの一般的なコードレビューに属します。その懸念事項については一度だけ言及し、先に進みます。
変更であって、コードベースではない
作成者は「これを悪化させたか?」と尋ねています。変更によって引き起こされたことを報告し、触れただけのことについては基本的に黙っていてください。3つまでの既存の発見項目は礼儀ですが、30個は別のレビューであり、誰も求めていません。
変更について意見を形成する前に、それを読んでください。記載された意図によって何が不完全かを判断し、差分を斜め読みすると、次のハンクで既に修正されたコードについての発見項目が生成されます。
基本原則
1. 最初に変更スコープを解決する
呼び出し全体がターゲットなので、/interface-review pr 482 はプルリクエスト482をレビューします。スコープ解決 には、受け入れられるターゲットとそれぞれの解決方法が記載されています。
ターゲットが指定されていない場合は、次の順序で解決し、最初に一致した時点で停止します。
HEADがgit merge-base origin/<デフォルトブランチ> HEADより先にある場合:その範囲 に加えて コミットされていない変更。コミット数とコミットされていないファイル数は別々に記載します。- ワーキングツリーがダーティである場合:コミットされていない変更。
- どちらでもない場合:レビューする変更はありません。停止し、「変更がない場合は、発明するのではなく尋ねる」に従って尋ねます。
順序は重要です。ワーキングツリーを先にチェックすると、1つの無関係なフォーマット編集が12コミットのブランチを覆い隠し、レポートは依然として完全なカバレッジを主張することになります。
ロックファイル、スナップショット、生成された出力、ベンダーコード、バイナリを除外し、除外したものを明記します。除外後の空のスコープは、異なるルートで同じ場所に到達します。
2. 変更がない場合は、発明するのではなく尋ねる
クリーンなツリーでマージベースより先に何もない場合、ユーザーは存在しない変更のレビューを依頼したことを意味します。自分勝手に HEAD~1..HEAD にフォールバックしてはいけません。最後のコミットはたまたまランディングしたもの(多くの場合マージコミットで、他人の作業)であり、それに関するレポートはユーザーが意図したものに関するレポートと区別がつきません。
見つけたリポジトリの事実を述べ、次にルートを提供して待ちます。レビューするものがない には、収集すべき事実が記載されています。
- 最後のコミット、
HEAD~1..HEAD、短いSHAと件名で指定。ユーザーが選択する前に何が得られるかを確認できます。 - ユーザーが指定するターゲット:
pr <n>、ブランチ、ref、または範囲。「最初に変更スコープを解決する」に従って解決します。 - リポジトリ全体のインターフェース監査。これは変更レビューではありません。このスキルのスコープブロック、ステータス、既存セクションなしで、リポジトリ全体のレビューとして
better-interfaceに委ねます。変更がない場合、すべての発見項目は既存であり、分類は何も言いません。
尋ねる前に、現在のブランチでオープンなプルリクエストがないか確認し、最初にそれを提示します。コミットが既にランディングしたブランチは変更なしと解決されますが、そのプルリクエストはまさにユーザーが意図したものです。
除外後にスコープが空になった場合は、除外したファイルを伝え、同じ方法で尋ねます。何もないレビューを「承認」として報告しないでください。
3. 差分はサーフェスではない
変更されたファイルは証拠であり、レビューの主題ではありません。そのブラストラディウスは、それがレンダリングされるサーフェスのセットです。それらのサーフェスをレビューします。
ブラストラディウスをデフォルトで1ホップ拡大します:直接のインポーターと呼び出し元。デザイントークン、テーマ値、共有プリミティブについては2ホップ拡大します。これらの場合、1行が製品全体に影響を与える可能性があります。
スコープ解決のルールに従って順序付けられた、最大5つのコンシューマーをレビューし、拡大しなかった数を明記します。境界のないスイープは、主張するカバレッジをサポートできず、明記されていないカットオフは完全性として読み取られます。
4. 削除された行を読む
リグレッションは変更後の状態では見えません。すべてのハンクの - 側を 削除されたシグナル と照らし合わせて読んでください。
シグナルは手がかりであり、発見項目ではありません。削除がリグレッションであるのは、変更内の何もそれを置き換えておらず、その判断はドメインスキルが担当する場合のみです。一致しない削除をそれぞれその所有者にルーティングし、そのスキルが確認したものだけを報告し、ステータスを Regression にします。これにより、作成者は新しい間違いを犯したのではなく、以前は機能していたものを壊したことを知ることができます。
5. すべての発見項目を分類する
すべての発見項目に次のいずれかのステータスを割り当てます。
Introduced: 変更によって作成されました。Regression: 変更によって以前は正しかったものが弱体化しました。Pre-existing: 変更によって触れられたコードに存在するが、この変更によって引き起こされたものではありません。
ステータスは、変更がどのファイルにあるかではなく、差分が触れたものによって決定します:変更が触れたことのない行は、ハンクから3行離れていても Pre-existing です。重要な場合はベース ref に対して確認します:
git blame -L <行>,<行> "$BASE" -- path/to/file
すべての発見項目をステータス付きで引き渡し、better-interface が上限と判定ルールを適用できるようにします。
6. 変更をその表明された意図に照らして評価する
プルリクエストのタイトルと本文、リンクされた issue、コミットメッセージを読み、その後インターフェースが主張を実現しているかをレビューします。
これにより、不完全な変更が明らかになります。サーフェスレビューではそれが見えません。存在する状態を検査するためであり、ここでのポイントは存在しない状態です。
- 新しいバリアント、サイズ、テーマが一部の状態にのみ適用されている:ホバー、フォーカス、アクティブ、無効、ローディング、選択。
- 新しいユーザー向け文字列が、プロジェクトが管理する翻訳カタログにエントリーがない。
- 新しいコンポーネントに空の状態、ローディング状態、エラー状態、無効状態、または狭い幅の状態がない。
- 既にそのピアを保持している兄弟には追加されていないサーフェスにコントロールが追加された。
スコープクリープを報告しないでください。変更がやりすぎかどうかはプロセスの問題であり、インターフェースの問題ではありません。
7. レビューを better-interface に委ねる
better-interface に、スコープブロック、影響を受けるサーフェス、およびすべての発見項目のステータスを引き渡します。better-interface はドメインスキルにルーティングし、重大度を適用し、統合し、上限を強制し、判定を発行します。
better-interface が利用できない場合は、解決されたスコープとファイルのインベントリを報告し、不足しているスキルとして名前を挙げて停止します。重大度スケール、上限、または判定を発明してはいけません。
8. ワーキングツリーを決して変更しない
変更レビューは読み取り専用です。これにはチェックアウトも含まれます。プルリクエストの ref をフェッチします。決してチェックアウトしてはいけません。git fetch は .git にのみ書き込むため許可されます。gh pr checkout、git checkout、git switch、git stash は作成者が開いているファイルを書き換えます。これらはローカル編集に対して失敗するか、破棄するため、許可されません。
レンダリングによる検証はオプトインです。プロジェクトが安価なプレビューを公開しているか、ユーザーがレンダリングレビューを要求した場合を除き、視覚的およびランタイムの主張を 未確認 とマークします。その場合は、分離されたワークツリー(git worktree add /tmp/review-<n> refs/remotes/pr/<n>)を使用し、完了したら削除します。
終了する前に
| ミス | 修正方法 |
|---|---|
| ブランチの代わりに1つの無関係な編集がレビューされた | ワーキングツリーの前に merge-base をチェックし、両方のカウントを報告する |
| 変更がないため最後のコミットがレビューされた | 事実を述べ、最後のコミット、名前付きターゲット、またはリポジトリ監査を提供する |
| ハンクがコンシューマーなしでレビューされた | 1ホップ拡大し、トークンとプリミティブの場合は2ホップ拡大し、スキップしたものを明記する |
差分の + 側だけが読まれた | - 側で削除されたアクセシビリティ、フォーカス、モーション、テキストシグナルを検索する |
| 同等の置換がリグレッションとして報告された | 削除をその所有者にルーティングし、確認したものだけを報告する |
| 削除が新しい間違いとして報告された | ステータスを Regression にして、以前は機能していたことを作成者に知らせる |
ハンクの近くの行が Introduced とステータスされた | 差分が触れたものに基づいてステータスを決定し、ベース ref に対する git blame で確認する |
| レビューするためにプルリクエストがチェックアウトされた | ref をフェッチし、その場でレビューする |
| レビュー対象の ref に存在しない行番号が引用された | スコープブロックに指定されたヘッド ref に対して引用する |
| 重大度スケールまたは発見項目の上限がここで再記述された | better-interface に委ねる |
| レポート内の正当性、テスト、セキュリティに関する発見項目 | 懸念事項を一度だけ述べ、プロジェクトのコードレビューを指して削除する |
レビュー出力形式
スコープブロックで開始します。
| フィールド | 値 |
|---|---|
| ターゲット | branch、working、staged、pr 482、または入力された範囲 |
| ベース ref | a1b2c3d の origin/main |
| ヘッド ref | e4f5g6h の refs/remotes/pr/482 |
| コミット | 7 コミット済み、2 ファイル未コミット |
| スコープ内ファイル | 除外後 12 |
| 除外 | pnpm-lock.yaml、src/__snapshots__/: ロックファイルとスナップショット |
| 拡大されたサーフェス | CheckoutPage、SettingsPanel; さらに3つの Button コンシューマーは拡大されず |
カバレッジテーブルは変更せずに続きます。変更スコープに証拠がないドメインは「レビューなし: 変更スコープに証拠なし」となり、これはギャップではなくカバレッジステートメントです。
次に発見項目。「すべての発見項目を分類する」に従って Status 列を付けます。
| 重大度 | ドメイン | ステータス | 場所 | 変更前 | 変更後 | 理由 |
|---|---|---|---|---|---|---|
| HIGH | アクセシビリティ | Regression | src/Dialog.tsx:42 | この変更で aria-label="Close" が削除された | アイコンのみのコントロールに aria-label="Close" を復元する | 閉じるコントロールはこの変更前はアクセシブルな名前を持っていたが、もはや持っていない |
Introduced または Regression の発見項目がない場合は、テーブルを省略し、「この変更には実行可能なインターフェース発見項目はありません」と記載します。
次に Pre-existing 発見項目。最大3つまで、重大度の高い順に、この変更の責任ではないと明確に記載します。ない場合はセクションを省略します。
| 重大度 | ドメイン | 場所 | 問題 |
|---|---|---|---|
| MEDIUM | タイポグラフィ | src/Toolbar.tsx:7 | 数値バッジがプロポーショナル数字を使用している; この変更より前から存在 |
上限と判定は Introduced と Regression のみを対象とします。Pre-existing 発見項目は上限の対象外であるため、レガシーファイルに触れてもファイル全体の監査にはなりません。また、判定の対象外でもあるため、発見項目が既存のみの変更は「承認」となります。
HIGH が残っている場合は Block で、そうでない場合は Approve で終了し、残りの発見項目はテーブルに作業として残します。better-interface が利用可能な場合、重大度スケールと上限はそれから取得します。