Skip to content
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

Add a flag to enforce commit signoff #73

Closed
johnboyes opened this issue Jan 11, 2021 · 17 comments · Fixed by #74
Closed

Add a flag to enforce commit signoff #73

johnboyes opened this issue Jan 11, 2021 · 17 comments · Fixed by #74

Comments

@johnboyes
Copy link

johnboyes commented Jan 11, 2021

Hi, it would be great to have an optional flag to enforce commit signoff, e.g. when working on a project which requires every commit to be signed off (see the git commit documentation)

This would involve checking for the presence of a Signed-off-by line (see example below) in the commit message.

Signed-off-by: Joe Bloggs <joe.bloggs@example.com>

@mristin
Copy link
Owner

mristin commented Jan 11, 2021

Thanks for the suggestion, @johnboyes! Do more such lines exist or is Signed-off-by: ... the only one?

If there are multiple such lines, it might make sense to come up with a more general interface where the user can specify that there should be lines conforming to a regular expression (including their cardinality). However, if signed-off-by is a single case, the flag seems like a better and more reusable solution across different projects (i.e., each project can just use the flag instead of copy/pasting the regular expressions).

Could you dig into the Git documentation to see if there are more such lines?

@johnboyes
Copy link
Author

johnboyes commented Jan 11, 2021

Hi @mristin, thanks for the quick reply!

For git commit signoff, it's just that one Signed-off-by: line. So as you say, a flag (called something like enforce-signoff, maybe) would work well.

NB the check would not need to enforce that the Signed-off-by: line is always the last line in the commit message, as that is not stipulated. So it's just a check that the line appears anywhere in the body of the message.

@johnboyes
Copy link
Author

Could you dig into the Git documentation to see if there are more such lines?

I've looked through the https://git-scm.com/docs/git-commit documentation and there's no other git commit flags that mandate certain content in the commit message that I can see 🙂

@mristin
Copy link
Owner

mristin commented Jan 11, 2021

@johnboyes So there should be a line matching:

^\*Signed-off-by:\s*[^<]+\s*<[^@>, ]+@[^@>, ]+>\s*$

, did I get that right? Or is it only the prefix "Signed-off-by:" what matters? I suppose you want to automatically process the git log at some point? Does it have to be an e-mail address in parentheses?

@johnboyes
Copy link
Author

johnboyes commented Jan 11, 2021

Thanks @mristin, it's the format of the whole line that matters - in pseudocode it needs to be

Signed-off-by: A Name <emailaddress>

The name doesn't need to be a certain number of words.

The email address needs to be a valid email format, enclosed by angle brackets.

My regex game isn't strong enough to know if the regex is exactly right by looking at it, apologies ;-)

@johnboyes
Copy link
Author

johnboyes commented Jan 12, 2021

Some examples:

Valid
Signed-off-by: Random J Developer <random@developer.example.org>
Invalid
Signed-off-by Random J Developer <random@developer.example.org>
signed-off-by: Random J Developer <random@developer.example.org>
Signed-off-by: Random J Developer <randomdeveloper.example.org>
Signed-off-by: Random J Developer (random@developer.example.org)
Signed-off-by: Random J Developer random@developer.example.org
Signed off by: Random J Developer <random@developer.example.org>
Signed_off_by: Random J Developer <random@developer.example.org>

@mristin
Copy link
Owner

mristin commented Jan 12, 2021

Thanks, @johnboyes! I hope to be able to have a look at it tonight.

mristin added a commit that referenced this issue Jan 12, 2021
This patch adds a flag to enforce a sign-off of a commit. See [1] for
more information.

Fixes #73.

[1]: https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff
mristin added a commit that referenced this issue Jan 12, 2021
This patch adds a flag to enforce a sign-off of a commit. See [1] for
more information.

Fixes #73.

[1]: https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff
@mristin
Copy link
Owner

mristin commented Jan 12, 2021

@johnboyes I need to get my Linux machine to re-build the package as I could not get the dependencies installed on my Windows machine. I'll bump the minor version tomorrow.

@johnboyes
Copy link
Author

Thanks @mristin. That would be really great if you can create a new release for this. May be worth adding an overview of this functionality in the README, btw. Thanks again.

@mristin
Copy link
Owner

mristin commented Jan 15, 2021

Hi @johnboyes,

That would be really great if you can create a new release for this.

The new version 2.3.0 is up -- please let me know if it works as expected for you.

May be worth adding an overview of this functionality in the README, btw.

Fixed in #77, thanks for spotting it! I forgot to document the feature in haste.

@johnboyes
Copy link
Author

johnboyes commented Jan 17, 2021

Hi @mristin

The new version 2.3.0 is up -- please let me know if it works as expected for you.

Thanks for this. I just tried it but couldn't get it to work I'm afraid - after turning on the check I couldn't get it to flag commits with no sign-off.

@mristin
Copy link
Owner

mristin commented Jan 17, 2021

Thanks for checking it out, @johnboyes ! Could you please send me the relevant snippet of your workflow so that I can try to reproduce the error?

@mristin mristin reopened this Jan 17, 2021
@johnboyes
Copy link
Author

johnboyes commented Jan 17, 2021

Sure thing @mristin, here's a quick example

Apologies if it's user error on my part!

@mristin
Copy link
Owner

mristin commented Jan 17, 2021

Did you maybe forget to copy/paste the full URL? (The link to the example looks like a commit hash to me.)

@johnboyes
Copy link
Author

I fixed the link within 20 seconds of posting it, you must have been quick ;-)

@mristin
Copy link
Owner

mristin commented Jan 18, 2021

@johnboyes Could you please try the verision 2.3.1 on your repository? The end-to-end test passes on my side, but you never know :).

For some reason, the distribution files included in the release 2.3.0 were not properly built, but I didn't notice it. There will be more thorough manual end-to-end tests from now on to avoid such issues.

@johnboyes
Copy link
Author

Hi @mristin, yes that all works properly now, thanks. Doing end-to-end tests every time is a great idea, I wonder if you could automate them? You could have a GitHub Action workflow in your repo triggered on every push, that programatically creates commits on an example repo, running against the corresponding version of the opinionated-commit-message checker? Just sharing ideas :-)

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 a pull request may close this issue.

2 participants