fix(ci): lint integration commits on dispatch - #1806
Conversation
workflow_dispatch admits an integration branch and previously skipped commitlint, so batch-fold follow-ups with headers of 73, 75, and 85 characters landed in #1796 and failed the master push. Dispatch now lints the commits a merge onto the default branch would introduce. header-max-length stays 72. A dispatch of the default branch does not rejudge published history. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a943654d99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| base=$(git -C "$repository" merge-base "$head" "$upstream") | ||
| lint_range "$base" |
There was a problem hiding this comment.
Use the resolved upstream ref itself as the left side of the lint range. When the dispatch head and default branch have a criss-cross history, they can have multiple best common ancestors; git merge-base -h explicitly says that --all outputs all common ancestors, while this invocation selects only one. Consequently, base..head can include a different common ancestor that is already published on the default branch and reject the dispatch for its old message, contrary to the stated no-rejudging behavior. upstream..head directly selects exactly the commits reachable from the proposed head but not from the default branch.
Useful? React with 👍 / 👎.
2 participants
Summary
workflow_dispatchnow lints the commits a merge onto the default branch would introduce, using the same commitlint rules a master push uses.header-max-lengthstays 72. The workflow trigger list is unchanged: push andworkflow_dispatchonly.Motivation
#1796 folded batch B through
workflow_dispatch, then the master push (CI run 35423924082, tip3f71bccb2dba) failedheader-max-lengthon three follow-up commits:92b22f71996ff40ec21c2de08d4f53cd63cea6f8(73)4c5275f31a54e8b2d220db120223c16609bd368e(75)943f641111e71050a1c487af7e6f53e1999a482c(85)Dispatch was the admission path and did not run commitlint. Rewriting those published SHAs is left to the tip-commit-lint lane; this change does not widen ignores or the 72-character rule. A dispatch of the default branch has an empty range, so published history is not rejudged.
Changes
scripts/lint-ci-commits.shselects the pushbefore..HEADrange or the dispatchmerge-base(default, HEAD)..HEADrange, then callsscripts/lint-commit-range.mjs..github/workflows/ci.ymlruns that script for push andworkflow_dispatch.scripts/test-lint-commit-range.pycovers the three failing subjects, the same follow-ups once the header fits, a default-branch dispatch that must not rejudge, and the existing push path.Test plan
python3 scripts/test-lint-commit-range.py— 9 tests, OKb4aa02ae61..3f71bccb2dexits 1 with those three SHAs and lengths 73 / 75 / 850f94ec9f9bagainstb4aa02ae61exits 1 on the same three SHAs3f71bccb2dagainst itself exits 0cargo nextest run --workspace --no-fail-fastnot run; this change does not touch Rustcargo clippynot run; this change does not touch RustCommit:
a943654d99f0ed61a4f8b15aff278562a41fbeb7Checklist
.envfiles includedCHANGELOG.mdupdated — not applicable; CI lint entry point only