-
Notifications
You must be signed in to change notification settings - Fork 40
Introduce a Git hook to help ensure commits are signed-off #126
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Philip John <phil@philipjohn.me.uk>
Signed-off-by: Philip John <phil@philipjohn.me.uk>
Signed-off-by: Philip John <phil@philipjohn.me.uk>
|
I'm generally against bundling this into the repo (eg with things like Husky), but I think a manual command is an acceptable tradeoff. (Note though, this requirement is true for all repositories on this organisation, and indeed as a general rule across LF projects.) Should we add a prepare-commit-msg hook as well? Git does intentionally not have a config flag for this as "Adding the Signed-off-by trailer to a patch should be a conscious act and means that you certify you have the rights to submit this work under the same open source license.", but I know a lot of people are using it. |
|
I think having this as a check will help make sure people actually do the thing before we get to the part where we have to tell them "You need to rebase the PR so we can accept it." Better link for Ryan's link - https://stackoverflow.com/a/46536244 @philipjohn Any thoughts? |
| }, | ||
| "scripts": { | ||
| "setup": [ | ||
| "./bin/install-git-hooks.sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I wonder if this would be better suited for
package.jsonsince that is the canonical runner of scripts. -
There is
git config --local core.hooksPath .githookswhich would set the hooks directory and avoid the need to symlink files. This makes it work across operating systems and it is the same logic that husky uses behind the scenes. It makes is clear which hooks are available and can be enabled.
| #!/bin/sh | ||
|
|
||
| # Ensure the commit is signed-off by the author. | ||
| if ! grep -q '^Signed-off-by: ' "$1"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we rewrite this in NodeJS to remove the Bash dependency? All of the tooling already requires Node so that would work out of the box for everyone consistently.
Adds a commit-msg hook that checks for the presence of the "Signed-off-by: " string in the commit message and rejects the commit when it's missing. Additonally, it checks if there is more than one "Signed-off-by: " string and rejects that too (this is from the default hook sample).
To make life easier for contributors a composer script is added so that running
composer run setupruns a simple bash script (bin/install-git-hooks.sh) which adds a symlink from the committed git hook to.git/hooks/commit-msg.Finally, the docs are updated to let people know they can install the git hook. I opted to leave the manual instructions for signing commits in place, as it's helpful for understanding as well.