From 51ca9d22d0980a21ebd2d9e86b1b67dde414db90 Mon Sep 17 00:00:00 2001 From: Matthew Pharr Date: Mon, 19 Oct 2026 11:00:17 +0200 Subject: [PATCH] CI - FEATURE - Support stacked pull requests in the test and harness-stamp checks Run the test suite on pull requests of any base, and check a stacked pull request's harness stamp against its own src/ changes rather than the whole tree, so merging an updated parent in does not invalidate it. Pull requests targeting develop or main keep the strict check. Document the stacked workflow and the base-branch harness baseline. Co-Authored-By: Claude Opus 5.5 --- .github/PULL_REQUEST_TEMPLATE.md | 3 +++ .github/workflows/pr-conventions.yaml | 17 +++++++++++++++++ .github/workflows/test.yaml | 5 ++--- docs/development/git-workflow.md | 10 ++++++++++ docs/development/naming.md | 2 +- docs/development/regression-harness.md | 4 ++++ 6 files changed, 37 insertions(+), 4 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1a906d51fd..f1d9f2b715 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -25,6 +25,9 @@ checkout and a mid-run edit silently mixes code versions (see the doc's "Run iso git fetch && git checkout develop && git merge --ff-only origin/develop regress --cases diiid_n1 --refs develop, + +Stacked on another PR? Use that PR's branch as the baseline, --refs , +(docs/development/git-workflow.md, "Stacked pull requests"). --> ``` diff --git a/.github/workflows/pr-conventions.yaml b/.github/workflows/pr-conventions.yaml index 93103e2e99..7e688b1032 100644 --- a/.github/workflows/pr-conventions.yaml +++ b/.github/workflows/pr-conventions.yaml @@ -68,6 +68,8 @@ jobs: - name: Is the regression stamp still current? if: steps.scope.outputs.touches_src == 'true' env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | stamp=$(python3 ci/conventions/check_subject.py --harness-sha "$RUNNER_TEMP/pr_body.md") @@ -75,6 +77,21 @@ jobs: echo "::error::Harness stamp '$stamp' is not a commit in this repository." exit 1 fi + + # A stacked PR's report covers only its own changes, so it stays current until + # the PR's own src/ patch changes; merging an updated parent in does not stale it. + # -U0 keeps parent edits that only shift the patch's context lines from counting. + if [ "$BASE_REF" != "develop" ] && [ "$BASE_REF" != "main" ]; then + own_src_patch() { git diff -U0 "$(git merge-base "$BASE_SHA" "$1")" "$1" -- src/ | git patch-id --stable | cut -d ' ' -f 1; } + if [ "$(own_src_patch "$stamp")" != "$(own_src_patch "$HEAD_SHA")" ]; then + echo "::error::The regression report is stale: this PR's own changes under src/ differ from those at $stamp." + echo "Re-run the harness against $BASE_REF and update the '_(harness @ ...)_' stamp in the release-note block." + exit 1 + fi + echo "Stacked on $BASE_REF: this PR's own src/ changes match those at $stamp." + exit 0 + fi + changed=$(git diff --name-only "$stamp".."$HEAD_SHA" -- src/) if [ -n "$changed" ]; then echo "::error::The regression report is stale. Files under src/ changed after $stamp:" diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a5e3561d53..596e565dea 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -1,10 +1,9 @@ name: Build and Test Julia Package on: + # Every pull request, whatever its base, so a PR stacked on another feature branch + # is tested too; it is tested again once retargeted, when it syncs with develop. pull_request: - branches: - - main - - develop push: branches: - main diff --git a/docs/development/git-workflow.md b/docs/development/git-workflow.md index e2da1c2240..cb3f82f7f1 100644 --- a/docs/development/git-workflow.md +++ b/docs/development/git-workflow.md @@ -27,6 +27,16 @@ Examples: `bugfix/sing-lim-bounds-error`, `feature/kinetic-damping`, `performanc Author-named branches (e.g. `jmh/`, `nlogan/`) are not used — git history already records authorship on every commit. +## Stacked Pull Requests + +A change that builds on another unmerged pull request can be opened against that pull request's branch instead of `develop`. When the lower pull request merges, its branch is deleted and GitHub retargets the one above it to `develop`. + +- **Update by merging, not rebasing.** Merge the parent branch into yours when it moves. A rebase rewrites the commit your harness stamp points at, and CI can no longer find it. +- **Merge the lower pull request with a merge commit.** A squash or rebase merge forces every pull request above it to rebase. +- **Run the harness against the base branch:** `regress --cases diiid_n1 --refs ,`, so the report and the release note's **Numerical impact** cover only this pull request. + +CI runs the test suite on every pull request whatever its base. While the base is another feature branch, the harness stamp stays current until this pull request's own `src/` changes differ from those at the stamped commit, so merging an updated parent in does not invalidate it. Once retargeted to `develop`, the usual check applies: nothing under `src/` may differ from the stamped commit. Because the check compares file contents, a report taken on top of the final parent survives the retarget unless `develop` has since gained other `src/` changes. + ## Hotfix Workflow Hotfixes address critical bugs in production (`main`) that cannot wait for the next release cycle: diff --git a/docs/development/naming.md b/docs/development/naming.md index 5aca2fedf2..16763ac6d2 100644 --- a/docs/development/naming.md +++ b/docs/development/naming.md @@ -102,7 +102,7 @@ Every PR body carries one: These three fields are the things a reader of the diff cannot work out for themselves, which is why they are asked for rather than inferred. -**Numerical impact** comes from the regression harness, which every PR must run anyway (see [`regression-harness.md`](regression-harness.md)). Record the commit it ran at. Review changes code, and a report from five commits ago may no longer be true — so CI re-checks on every push and fails if anything under `src/` changed after the stamped commit. Commits touching only docs, tests, or examples never trip it. +**Numerical impact** comes from the regression harness, which every PR must run anyway (see [`regression-harness.md`](regression-harness.md)). Record the commit it ran at. Review changes code, and a report from five commits ago may no longer be true — so CI re-checks on every push and fails if anything under `src/` changed after the stamped commit. Commits touching only docs, tests, or examples never trip it. A PR stacked on another PR's branch is checked more narrowly — only its own `src/` changes must match those at the stamp — and strictly again once it targets `develop` (see [`git-workflow.md`](git-workflow.md#stacked-pull-requests)). **Migration** must say something real when the title carries `!`. diff --git a/docs/development/regression-harness.md b/docs/development/regression-harness.md index ff0405109a..0dab467f41 100644 --- a/docs/development/regression-harness.md +++ b/docs/development/regression-harness.md @@ -141,6 +141,10 @@ regress --cases diiid_n1 --refs develop,my-feature-branch what keeps the baseline current (or pass `origin/develop` directly). After a fetch, a local pointer that still lags its remote prints a `!! STALE BASELINE` banner; without one the banner is silent. +A pull request stacked on another one compares against the branch it targets, not `develop`, so +its report shows only its own changes: `regress --cases diiid_n1 --refs ,my-feature-branch` +(see [Stacked pull requests](git-workflow.md#stacked-pull-requests)). + `local` stays the right tool for a quick spot check on uncommitted work, provided you leave the tree alone until it finishes.