docs: PRレビュー時のスコープ照合ルールを追加 (#57)#60
Open
strnh wants to merge 2 commits into
Open
Conversation
issue #57 の調査結果: - PDF取込機能は PR #56 には含まれておらず、専用の PR #51 (Issue #1) で 2026-07-07 に別途マージ済み（git diff 68a7eca..65b265a および gh pr diff 56 --name-only で PDF/Import 関連 0 件を確認） - 「PR #56 に混在」に見えたのは、古いローカル main（68a7eca 未取得） を比較基準にしたことによる誤認 再発防止として、レビュー時に最新 origin/main を基準にした スコープ照合（PR本文 vs 実 diff）を行う運用ルールを明文化する。 Closes #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
issue #57 の調査結果（PR #56 への誤認）を踏まえ、PRレビュー時に「PR本文の宣言スコープ」と「実際の差分ファイル」を突合して確認する運用ルールを AGENTS.md に明文化するPRです。レビューの比較基準が古いローカル main だと誤認が起きうる、という再発防止の手順を追加しています。
Changes:
- PRレビュー前に
git fetch originを行い、比較基準を最新にする旨を追記 - PR本文スコープと
gh pr diff --name-onlyのファイル一覧を突合する手順を追記 - 独立機能の混在を避け、必要なら別issue/別PRへ切り出す運用を追記
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## PRレビュー時のスコープ照合 | ||
|
|
||
| - レビュー前に `git fetch origin` を実行し、diff の比較基準は必ず最新の `origin/main`（merge-base）にする。ローカル main が古いと、他PRでマージ済みの変更が「このPRの新規追加」に見える（issue #57 の誤認の原因）。 |
Owner
Author
There was a problem hiding this comment.
指摘のとおり修正しました（80f0952）。「merge-base（共通祖先コミット）を基準に diff を取る」と表現を正確化し、具体例として git diff origin/main...HEAD（= git diff $(git merge-base origin/main HEAD) HEAD と同等）を明記しました。
Copilot レビュー指摘に対応: merge-base は origin/main 自体ではなく 共通祖先コミットであるため、表現を修正し `git diff origin/main...HEAD` の具体例を追記。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| ## PRレビュー時のスコープ照合 | ||
|
|
||
| - レビュー前に `git fetch origin` を実行し、diff は最新の `origin/main` と PR ブランチの merge-base（共通祖先コミット）を基準に取る（例: `git diff origin/main...HEAD`、これは `git diff $(git merge-base origin/main HEAD) HEAD` と同等）。ローカル main が古いと、他PRでマージ済みの変更が「このPRの新規追加」に見える（issue #57 の誤認の原因）。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要 (Summary)
issue #57「PR #56: PDF取込機能が仕切書機能と無関係なスコープで混在している」の調査と対応。
Closes #57
調査結果 (Investigation)
issue の前提を検証した結果、PDF取込機能は PR #56 のマージ差分に含まれていないことを確認した。
app/Support/QuotePdfParser.php/config/import.php/ImportControllerのPDF対応 /Import.tsx/tests/Fixtures/*.pdfと関連テスト）は、専用の PR Issue #1: quotebuilder出力PDFの取込に対応 #51（Issue pdf取込の再検討（座標ベース or 別ライブラリ） #1、2026-07-07 マージ、コミット 68a7eca）で main に取り込み済み。PR Issue #1: quotebuilder出力PDFの取込に対応 #51 本文にスコープ・採用方式・検証内容が記載されている。git diff 68a7eca..65b265aは仕切書関連の 31 ファイルのみで、PDF/Import 関連は 0 件。gh pr diff 56 --name-onlyでも PDF/Import 関連ファイルは 0 件。「PR #56 に混在して見えた」のは、PR #51 マージ（68a7eca）を取得していない古いローカル main を比較基準にしたことによる誤認と考えられる（PR #56 ブランチは途中で origin/main をマージ（2d63620）しており、古い基準と比較すると PR #51 由来のファイルが「このPRの新規追加」に見える）。
対応内容 (Changes)
issue の対応案 3 点に対して:
AGENTS.mdに運用ルールを明文化git fetch originし、比較基準は必ず最新origin/main（merge-base）gh pr diff --name-onlyの実ファイル一覧を突合影響範囲 (Impact)
🤖 Generated with Claude Code