ci: only check co-author trailers on commits the PR adds - #576
Open
lmo-selected wants to merge 1 commit into
Open
ci: only check co-author trailers on commits the PR adds#576lmo-selected wants to merge 1 commit into
lmo-selected wants to merge 1 commit into
Conversation
The trailer check walks `git rev-list "$base..$HEAD_SHA"`, which includes every commit reachable from the head, not only the ones the PR contributes. When a PR merges another branch in, the merged-in commits enter that range and are checked as if the author had written them. That makes the check fail on history nobody in the PR can change. A branch that merges main in picks up main's commits; the check then reports an offending trailer on a commit that is already on main, and the only ways out are rewriting shared history or merging red. Walking first-parent restricts the range to the commits the PR itself adds. A commit that introduces a banned trailer is still caught, whether it is an ordinary commit or a merge commit carrying the trailer in its own message, because both sit on the first-parent path. Only the contents of a merged-in branch are skipped, which is what the check was never meant to police.
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.
The
Co-author trailerscheck walks every commit reachable from the head:That range is not "the commits this PR adds" — it also contains everything a merge brought in. So a PR that merges another branch into itself is checked against that branch's commits as well as its own.
The failure mode is a check that cannot be satisfied. Merge
maininto a branch and the range picks upmain's history, including9ff90fc("credits: record co-authors the squash merges dropped"), which records 13@users.noreply.github.comtrailers. The check flags it, and the author's options are rewriting published history or merging red. Neither is a review signal.--first-parentrestricts the range to the commits on the PR's own line of development.It still catches what it is meant to catch
Both offending shapes sit on the first-parent path, so neither is affected:
Only the contents of a merged-in branch are skipped, which is history that entered through whatever review that branch had. The added stderr line says so, so someone reading a failure knows the scope.
Repro
git rev-list "$base..$HEAD_SHA"upstream: credit contributorsgit rev-list --first-parent "$base..$HEAD_SHA"Controls, both still flagged under
--first-parent: a branch whose own commit adds a banned trailer, and a branch whose merge commit carries one in its own message.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.