Skip to content

Add githooks to manage commit DCO sign-off#1932

Merged
alanpeixinho merged 2 commits into
kernelci:mainfrom
profusion:chore/hook-dco
Jun 12, 2026
Merged

Add githooks to manage commit DCO sign-off#1932
alanpeixinho merged 2 commits into
kernelci:mainfrom
profusion:chore/hook-dco

Conversation

@mentonin

@mentonin mentonin commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Adds a local commit-msg git-hook to check for commit messages without the DCO "Signed-off-by:" trailer.

Closes: #1922
Related: #1930, #1931

Caveats

@bhcopeland

Copy link
Copy Markdown
Member

Looks good thanks for this. To me but I'd probably stay away from "auto filling DCO" and leave that to user, but block the pipeline. Having CI adjust the commit + autofill seems risky to me.

@mentonin

Copy link
Copy Markdown
Contributor Author

The way I set this up, it has 2 hooks:

  1. The prepare-commit-msg hook adds the sign-off if an editor will open later to edit the commit message (i.e. it never changes your commit silently).

  2. The commit-msg hook checks if there is a DCO and blocks the commit otherwise, with an error message suggesting --signed|-s usage

The CI never adjusts the commit, unless I misunderstand something about how GH handles git hooks. If you think the prepare-commit-msg hook is still too risky, which would you like best:

  1. Remove it completely
  2. Keep it, but move documentation out of general setup and better explain the consequences
  3. Comment out the auto-added sign-off, adding convenience but requiring a manual action
  4. Add comments around the line reminding the user what the DCO is, why the comment shows up, or to use --signed|-s

@bhcopeland

Copy link
Copy Markdown
Member

Apologies I read it too quick, yes this is a local git hook. I'd drop this though still and go for 1. Just keep the ci check in place (and leave users to themselves).

Comment thread .pre-commit-config.yaml Outdated
args:
- -c
- |
if ! ( git interpret-trailers --parse "$1" | grep -q "Signed-off-by:" "$1" ); then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to remove the trailing $1 here. This will read it from stdin and exit 0 if it exists.

Comment thread .pre-commit-config.yaml Outdated
language: system
always_run: true

- id: prepare-dco

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I am with @bhcopeland, might be safer if we just use the pre commit check for DCO, and leave the user to add it by themselves.
I am not sure if we have permissions to add check on CI as well, but if we do, might be a good idea.

mentonin added 2 commits June 10, 2026 17:09
Signed-off-by: Luiz Georg <luiz.georg@profusion.mobi>
Signed-off-by: Luiz Georg <luiz.georg@profusion.mobi>
@mentonin

Copy link
Copy Markdown
Contributor Author

Removed the prepare-commit-msg hook and fixed code, ready for another review

@bhcopeland

Copy link
Copy Markdown
Member

LGTM :)

@alanpeixinho alanpeixinho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alanpeixinho alanpeixinho added this pull request to the merge queue Jun 12, 2026
Merged via the queue into kernelci:main with commit 1e4d2a9 Jun 12, 2026
7 checks passed
@mentonin mentonin deleted the chore/hook-dco branch June 14, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: add Git hook for Signed-off-by commit trailer

3 participants