Skip to content

Compare a pull request with the branch it targets - #9

Merged
mbbill merged 3 commits into
mainfrom
claude/review-base-stacked
Oct 11, 2026
Merged

mbbill merged 3 commits into
mainfrom
claude/review-base-stacked

Conversation

@mbbill

@mbbill mbbill commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Change

review-base.sh picked merge-base origin/main HEAD for every pull request. A pull request stacked on another work branch was therefore compared with main, and design-ready required its newest log entry to cover the parent branch's tree edits too, which it cannot do honestly.

For pull_request events the base is now the fork point from origin/$GITHUB_BASE_REF (main when the variable is unset). The script fails with a clear message when that branch was not fetched. Pushes and runs on main are unchanged.

Callers need no change. actions/checkout with fetch-depth: 0 already fetches the target branch, and GitHub sets GITHUB_BASE_REF on pull_request events.

Validation

python3 -B -m unittest discover -s . -p 'test_lint.py': 24 tests pass. It ran locally because it is a small check that does not depend on the compiler.

New cases, each failing on the old script:

  • A stacked pull request whose own change is logged resolves to the parent branch's tip and passes --require-approval.
  • The same stack with no new log entry still fails.
  • A missing target branch fails with its name.

The existing main-target case still passes.

Review

Independent read-only review: Codex (GPT-6.1 Sol), b1d184a..bda3a3a. It covered the shell, the tests, the README and the fetch behavior of actions/checkout. Two findings, both fixed:

  • The script checked refs/remotes/origin/<target> but then resolved the shorthand origin/<target>, so a tag of that name could shadow the branch. It now uses the full ref (ec1b04a). A local check with a shadowing tag picks the branch.
  • The missing-log test accepted any lint failure. It now asserts the missing-log diagnostic (ec1b04a, c8eee7c).

The reviewer's sandbox could not create temporary directories, so it ran no test bodies. The suite passes at c8eee7c (24 tests) and in CI.

Adoption

After merge, Snowghost-wf and Whitefoot move their design/skill pin.

🤖 Generated with Claude Code

Bai Ming and others added 3 commits October 11, 2026 15:44
A pull request stacked on another work branch was compared with main, so
design-ready charged it with its parent's tree edits and required its own log
entry to name them. review-base.sh now takes the fork point from
GITHUB_BASE_REF for pull_request events (main when unset) and fails when that
branch was not fetched.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mbbill
mbbill marked this pull request as ready for review October 11, 2026 23:00
@mbbill
mbbill merged commit 3bf89d7 into main Oct 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant