Repository navigation
ci: run the DCO check on every v2 PR now, before push, and after merge (#2616) #2619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2c67704
ci: run the DCO check on every v2 PR now, before push, and after merg…
cliffhall 38862e6
docs(dco): describe the PR check's scope as every v2 PR, stacked ones…
cliffhall 702001e
ci(dco): scope the PR job to v2/** bases rather than excluding main a…
cliffhall fa6538c
docs(dco): stacked-branch check and repair guidance, and refresh the …
cliffhall 15762ac
fix(dco): match GitHub's web-flow identity whole, and document the sq…
cliffhall 8ee5754
docs(dco): the push backstop does not cover a PR that edits the check…
cliffhall 8448039
fix(dco): local:dco excludes origin/main, so the gate passes on a mil…
cliffhall File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,26 +95,38 @@ previous one, not all cut from `v2/main`. | |
|
|
||
| **The `DCO` check fails the PR on any unsigned commit.** It is this repo's own | ||
| job (`.github/workflows/dco.yml` → `scripts/dco-check.mjs`, #2566), run on every | ||
| PR targeting `v2/main`, and it requires each commit to carry a `Signed-off-by: Name <email>` | ||
| v2 PR, stacked PRs included, and it requires each commit to carry a `Signed-off-by: Name <email>` | ||
| trailer whose name **and** email match either the commit's author or its | ||
| committer (case-insensitively). Its only exemptions are merge commits and | ||
| committer (case-insensitively). One relaxation: a commit GitHub itself | ||
| committed (`GitHub <[email protected]>`, as on a squash merge, whose author | ||
| name GitHub takes from the profile) passes on an author **email** match alone. | ||
| Its only exemptions are merge commits and | ||
| bot-authored commits; there is no partial credit — one unsigned commit out of six | ||
| fails the whole check, and the job's output names each offending commit and the | ||
| repair below. | ||
|
|
||
| ⚠️ **It is a merge gate only because it is a _required_ status check** — a | ||
| ruleset setting, not something the workflow file can declare. The job runs on | ||
| `pull_request_target`, so its workflow is read from `main`: it reports on PRs | ||
| only once a milestone merge has carried it there (#2566). The probot DCO app | ||
| `pull_request`, from the PR's own ref, so it reports on every v2 PR — stacked | ||
| ones included, any `v2/**` base — with no wait for a | ||
| milestone merge (#2616). A second job, `DCO (v2/main push)`, | ||
| re-runs the check over every push that lands on `v2/main` — a backstop for | ||
| anything that merged without a passing PR check — so a red there means an | ||
| unsigned commit is already on the branch. The probot DCO app | ||
| it replaced was never required, so when the app was suspended its check simply | ||
| stopped appearing (after #1981) and nothing went red for two months. If the | ||
| `DCO` check is ever missing from a PR, treat that as the outage it is. | ||
|
|
||
| **Check before you push** — the same script runs locally against the range the | ||
| PR will show: | ||
| **Check before you push.** `npm run local:gate` already does it: its | ||
| `local:dco` stage runs the same script over `origin/v2/main..HEAD` (minus | ||
| anything already on `origin/main`), right after | ||
| `local:validate`. On a **stacked** branch that range covers the whole stack, | ||
| parents included, which is stricter than the child PR's own check (that one | ||
| runs against the parent's branch). To check on its own, against the range the | ||
| PR will show, pass the PR's actual base: | ||
|
|
||
| ```sh | ||
| npm run dco:check -- --base origin/v2/main | ||
| npm run dco:check -- --base origin/v2/main # or origin/<parent branch> when stacked | ||
| ``` | ||
|
|
||
| **Prevent it with `git commit -s`.** Two things that look like automation and are | ||
|
|
@@ -136,6 +148,12 @@ git rebase --rebase-merges --signoff origin/v2/main # the base the PR targets | |
| git push --force-with-lease | ||
| ``` | ||
|
|
||
| ⚠️ **On a stacked branch, rebase against the parent branch, never | ||
| `origin/v2/main`.** A `--signoff` rebase onto `origin/v2/main` rewrites every | ||
| parent commit too, which forks the child from its parent and breaks the stack. | ||
| If the unsigned commit is in the parent, repair the parent's own branch first, | ||
| then rebase the child onto the repaired parent. | ||
|
|
||
| `--rebase-merges` keeps any merge commit on the branch — without it the rebase | ||
| flattens them, silently dropping a conflict resolution that lives only in the | ||
| merge. Use `--force-with-lease` rather than `--force`, and only rewrite when you are the | ||
|
|
||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.