Stop release testing on Dependabot PRs and fix the release checklist - #264
Open
gomezzz wants to merge 2 commits into
Open
Stop release testing on Dependabot PRs and fix the release checklist#264gomezzz wants to merge 2 commits into
gomezzz wants to merge 2 commits into
Conversation
The pull_request trigger carried the comment "only releases and hotfixes target main". That is wrong: Dependabot targets the default branch, which is main, so a GitHub Actions version bump ran the full three-backend, three-Python suite. A job-level actor filter is the only place this can be expressed, since pull_request cannot filter on who opened it. The concurrency group was grouped by github.ref, which push and pull_request report differently for the same commit, so it never deduplicated the pair it was written to deduplicate -- the 0.6.0 release branch ran the matrix twice on every commit. Grouping by the commit under test instead. The release checklist gains three things learned shipping 0.6.0: closing keywords must be repeated before every issue number or GitHub closes only the first, Read the Docs does not build tags unless a version is activated in its dashboard, and the conda-forge bot bumps only version and sha256, so dependency changes -- and the setuptools floor the PEP 639 license expression needs -- must be carried over by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YP9oDLmw636HhPSAD6Nv3p
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…ng it The previous attempt at this did not work, for two reasons the review caught. A pull_request run uses the workflow file from the merge commit, and Dependabot branches off main, so an actor filter living on develop would not have taken effect until the next release shipped it to main. And the SHA-based concurrency group only suppresses a duplicate while the first run is still in progress; the release checklist verifies the push run is green *before* opening the PR, by which point nothing is left to cancel, so the matrix ran twice anyway. Dropping the pull_request trigger removes both problems at once, along with the need for the actor filter. Release branches already match the push trigger, and a push run reports as a check on the PR opened from that branch, so nothing is lost except the duplicate. A hotfix branch not named release-* now needs a manual dispatch, which the checklist and the CI docs both say. Grouping by ref becomes correct again once only one event type reaches a branch, so it goes back to cancelling superseded pushes. Dependabot gets target-branch: develop. Targeting main violated the branching model outright -- main takes releases, hotfixes and doc fixes only -- and every merged action bump put main ahead of develop, which the first step of the release checklist requires not to be the case. docs/source/ci_cd.rst still described the pre-0.6.0 triggers; it now matches. The closing-keyword item moves next to the bullet that opens the PRs, since that is when the body is written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YP9oDLmw636HhPSAD6Nv3p
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.
Process defects found while shipping 0.6.0. No library code, no docs examples.
release_testing.ymldrops itspull_request: branches: [main]trigger.That trigger ran the full three-backend × three-Python matrix a second time on
every release commit, and also fired on Dependabot, which targets
mainbecause
mainis the default branch. Release branches already match thepushtrigger, and a push run reports as a check on the PR opened from that branch,
so nothing is lost. A hotfix branch not named
release-*now needs a manualdispatch — stated in both the checklist and the CI docs.
github.ref. With one event type perbranch that is correct again, and it recovers cancellation of runs superseded
by a newer push.
target-branch: "develop". This is the actual root cause.Targeting
mainviolates the branching model (maintakes releases, hotfixesand doc fixes only), and every merged action bump leaves
mainahead ofdevelop— which step 1 of the release checklist requires not to be the case.PR build(deps): bump the github-actions group with 4 updates #261 is an open example. Note Dependabot reads this file from the default
branch, so the retarget only applies once it reaches
main.docs/source/ci_cd.rststill documented the pre-0.6.0 triggers. Now matches.must be repeated before every issue number (
Closes #1, #2, #3closes only Added environment.yml #1,which left four issues open in 0.6.0); Read the Docs does not build tags unless
a version is activated in its dashboard, which is why
/en/vX.Y.Z/has 404edfor every release including 0.5.0; and the conda-forge autotick bot bumps only
versionandsha256, so dependency changes — and thesetuptools>=77.0.3the PEP 639 license expression needs — must be carried over by hand.
Related to #258.
Numerical behaviour
None — no library code is touched.
What changed after review
The review found that both original fixes were ineffective, and it was right:
pull_requestrun uses the workflow file from the merge commit.Dependabot branches off
main, so an actor filter merged todevelopwouldnot take effect until the next release carried it to
main. My original testplan claimed the effect would be observable on merge; that was wrong.
cancel-in-progressonly suppresses a duplicate while the first run is stillrunning. The checklist verifies the push run is green before opening the PR,
so by then there is nothing to cancel and the matrix runs twice regardless.
Worse, when it did fire it would leave a cancelled
release-testscheck on therelease PR, which checklist §4 does not account for.
Removing the trigger solves both, and makes the actor filter unnecessary.
Test plan
release_testing.ymlparses as valid YAML: triggers are nowpush/workflow_dispatch/release, the concurrency group is back togithub.ref, and the job-levelifis gone..github/dependabot.ymlparses withtarget-branch: develop.sphinx-build -W -b html docs/sourcesucceeds with the updated CI page.main: the next weekly Dependabot PRshould open against
developand start norelease-testsjobs.