feat(pre-commit): enforce commit signing
Summary
Problem
From time to time, new contributors open PRs on yari that include unsigned commits, and it is not trivial to sign the existing commits.
Solution
Prevent committing unsigned commits by verifiying that commit signing is enabled in the pre-commit hook.
Screenshots
Before
$ git config commit.gpgSign 0
$ git commit --allow-empty -m 'test'
yarn install v1.22.19
[1/5] 🔍 Validating package.json...
[2/5] 🔍 Resolving packages...
success Already up-to-date.
✨ Done in 0.26s.
yarn run v1.22.19
$ /Users/claas/github/mdn/yari/node_modules/.bin/lint-staged
→ No staged files found.
✨ Done in 0.33s.
[main 4dff57424] test
After
$ git config commit.gpgSign 0
$ git commit --allow-empty -m 'test'
ERROR: Commit signing is NOT enabled!
Please note that we require that all commits are signed.
Please see the documentation about signed commits and how to sign yours on GitHub:
- https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification
- https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits
husky - pre-commit hook exited with code 1 (error)
How did you test this change?
See above:
- Disabled commit signing:
git config commit.gpgSign 0 - Tried to commit:
git commit --allow-empty -m 'test'
I'm a bit on the fence about this one: as I see it, the point of code signing is to ensure that code that claims to be committed by e.g. me, Leo McArdle, is actually committed by me, and can be trusted.
But if an anonymous contributor opens a PR, I don't see value in that commit being signed - what surely matters more is that whoever reviewed that code has some kind of verifiable cryptographic signature, and it's easier IMO to squash a set of commits before merging, signing them with your own key, than taking a contributor through the steps of setting up PGP.
In the past, there were a handful of cases where new contributors opened a PR with unsigned commits, and these were rarely trivial to resolve, mostly requiring rounds of back and forth. This PR avoids those situations by making new contributors set up commit signing before they do their first commit.
@LeoMcA Are you suggesting an alternative, to keep the status quo as is, or lifting the signed commits requirement?
these were rarely trivial to resolve, mostly requiring rounds of back and forth
My suggestion is that if a new contributor submits a PR with unsigned commits, we just sign it on their behalf, which requires no back and fourth.
This PR avoids those situations by making new contributors set up commit signing before they do their first commit.
I'm pretty sure I've seen a fair few PRs which weren't prettier-ed correctly, so I'm not sure this guarantees that commits will end up signed. I'm also not sure we want to block code contributions on the sometimes complex setup of GPG.
I'm on the fence here, but I think what would put me in the "yeah why not" camp would be the ability to turn this check off with the presence of a .nogpg file or something.
@LeoMcA Are you suggesting an alternative, to keep the status quo as is, or lifting the signed commits requirement?
Keep the signed commits requirement, but in cases where it would be easier, just sign the commits for a contributor - as our signature as reviewer is far more meaningful than there's as a first/second time contributor (past a handful of PRs it's probably worth guiding them through the process of setting it up - as their signature then attaches some meaning)
This pull request has merge conflicts that must be resolved before it can be merged.