governance: enforce DCO and document continuity checks#208
Conversation
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
m-khan-97
left a comment
There was a problem hiding this comment.
The DCO tooling itself is clean — check_dco.py uses subprocess with list args (no shell injection), reads messages via git show --format=%B, and the regex requires a real name + bracketed email. Tests cover the happy path and malformed trailers. Good, small, well-scoped.
One blocking issue: the check will fail on merge commits.
commits_between() runs git rev-list --reverse BASE..HEAD with no --no-merges, so every commit in the PR range is checked — including merge commits. A git merge origin/dev merge commit has the default message ("Merge remote-tracking branch 'origin/dev'…") with no Signed-off-by, so it fails the check.
This isn't hypothetical: TFT444 merged dev into #161 twice during that PR's review cycle. With this workflow active, those merge commits would have hard-failed the DCO gate, blocking a legitimate PR through no fault of the author's own commits. The standard GitHub DCO app skips merge commits for exactly this reason.
Fix is one line — add --no-merges to the rev-list (and ideally a test asserting a merge commit is skipped). Alternatively, if the team wants to standardize on rebase-only (no merge commits in PRs), that's fine too, but then it should be documented in docs/dco.md so contributors know not to git merge dev.
Non-blocking: heads-up that this makes DCO a required check for everyone going forward — the team (and I, on #188) haven't been using git commit -s, so everyone will need to adopt it. Worth a Discord note when this lands. The docs/dco.md amend/rebase instructions are good for that.
Happy to approve once merge commits are handled.
commits_between() checked every commit in the PR range including merge commits, which carry Git's own default message and no Signed-off-by trailer since they aren't the author's own work. This would have hard-failed the DCO gate on legitimate `git merge origin/dev` commits (e.g. the two that landed in #161's review cycle) through no fault of the contributor's own commits — GitHub's own DCO app skips merge commits for the same reason. Add --no-merges to the rev-list call and document the exemption in docs/dco.md so contributors know only their own commits need signing. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Summary
Implements the repository-controlled parts of #205.
Signed-off-bytrailer on every introduced commitValidation
git diff --checkpassesOwner confirmation still required
Issue #205 must remain open until an organization owner privately verifies that at least two people can administer issues, merge approved work, publish/recover releases, and recover critical organization/deployment access. Documentation cannot prove that underlying access state.
Advances #205