Skip to content

feat(core): add Signed-off-by rule #53

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

Merged

Conversation

adam-moss
Copy link
Contributor

Added a new rule to allow for commit messages to be validated for the Signed-off-by: syntax as added when doing git commit --signoff.

I think I've changed everything required for this, but please shout up if I've missed something.

@marionebl
Copy link
Contributor

Thanks @adam-moss!

Currently the tests are failing, specifically export-all-rules https://travis-ci.org/marionebl/commitlint/jobs/258568271#L651. Could you adapt your changes?

@adam-moss adam-moss force-pushed the feat/signed-off-by branch from 971f88a to 3e3b82d Compare July 28, 2017 16:09
@adam-moss
Copy link
Contributor Author

Balls, no idea how I managed to miss that 😭

Hopefully that should've sorted it 👍

Copy link
Contributor

@marionebl marionebl left a comment

Choose a reason for hiding this comment

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

Just one clarification/comment. This is awesomely mergeable for a first shot.

@@ -0,0 +1,17 @@
export default (parsed, when, value) => {
const input = parsed.raw;
Copy link
Contributor

Choose a reason for hiding this comment

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

According to my test Signed-off-by is added to the .body of a commit when using --signoff. Do you have a specific reason for using .raw here?

Copy link
Contributor Author

@adam-moss adam-moss Jul 29, 2017

Choose a reason for hiding this comment

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

Honestly? I went with raw as I was struggling to dump out what was in each part of parsed as ava seems to eat console.log calls (first time I've used ava so not sure of the proper way to insert simple debug statements).

If the commit is split into subject, body, footer I would actually expect the Signed-off-by: to be in the footer logically, if a body is present, or the body, if no body is present (which is what your test shows).

For example:

test(signoff): --signoff

this does something with git signoff
    
BREAKING
    xyz
    
Signed-off-by: Adam Moss <an@email.address>

--signoff puts it at the end of the commit message so I err'd on the side of caution with raw.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think your assessment is correct. Let's go with a solution where the check if the searched string is on the very last line of .raw. Please adapt accordingly and add a test cases with Signed-off-by found on other lines being ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All done

@adam-moss adam-moss force-pushed the feat/signed-off-by branch from 3e3b82d to 5cb4f16 Compare July 29, 2017 20:31
@adam-moss
Copy link
Contributor Author

adam-moss commented Jul 29, 2017

@marionebl right that's those additional changes in, happy to take suggestions for improving... can probably come up with a better regex tbh 👍

@adam-moss adam-moss force-pushed the feat/signed-off-by branch from 5cb4f16 to 022d705 Compare July 29, 2017 21:44
@adam-moss adam-moss force-pushed the feat/signed-off-by branch from 022d705 to 0ca3788 Compare July 29, 2017 21:45
@marionebl marionebl merged commit cefeb74 into conventional-changelog:master Jul 30, 2017
@marionebl
Copy link
Contributor

Thank you!

@marionebl
Copy link
Contributor

Released with version 3.1.0 🎉

@adam-moss
Copy link
Contributor Author

Nice one 👍

@adam-moss adam-moss deleted the feat/signed-off-by branch July 30, 2017 07:00
bpedersen pushed a commit to bpedersen/commitlint that referenced this pull request Oct 15, 2019
this object will replace the other variables, it is more easy to scale adding new properties.
I'll extend the info in a future documents about plugins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants