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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update conventional commit linter #658

Closed
bcoe opened this issue May 19, 2019 · 9 comments
Closed

feat: update conventional commit linter #658

bcoe opened this issue May 19, 2019 · 9 comments
Assignees
Labels

Comments

@bcoe
Copy link
Member

bcoe commented May 19, 2019

Hey @marionebl 馃憢

We've been making some slight tweaks to the conventional commit spec, as we march towards a v1.0.0 of the spec. Mainly introducing ! a shorthand for breaking changes.

I've also finally taken the time to write a preset based directly on conventionalcommits.org.

It would be slick if we could update commitlint to match conventional-changelog-conventionalcommits and conventionalcommits.org, or its conventional commits linter.

@marionebl
Copy link
Contributor

Hey @bcoe, thanks for reaching out! I'm rolling some scenarios around, input and thoughts on the following would be awesome:

  1. Do we update commitlint-config-conventional to match the new spec or create commitlint-config-conventionalcommits. I am leaning towards option 1.

  2. Do you see cases for teams to limit / validate things around the !? Still wrapping my head around the implications.

@bcoe
Copy link
Member Author

bcoe commented May 20, 2019

re: 1, I agree let's just update the existing commitlint-config-conventional.

re: 2, I think, with how I'm currently picturing ! will work, we just need to allow this as a valid character in the type/scope prefix; there shouldn't be any validation needed beyond this.

@byCedric
Copy link
Member

Allowing it in the types/scopes will work just fine I think. We might add rules like prefer-exclamation, to warn/error for the "old" BREAKING: ... notation right?

@byCedric
Copy link
Member

byCedric commented May 26, 2019

I'll close issue #632 in favour of this one!

@byCedric
Copy link
Member

byCedric commented Jul 28, 2019

Ok, so I double-checked the current conventional config. I don't think a lot has to be changed to match the spec. The only "biggest" (probably breaking) change is to use the conventional-changelog-conventionalcommits parser options. Besides that, I found some other parts that probably require some discussion.

Are there more types allowed than currently listed?
In the specification, I found that rule 2 and rule 3 are more "user-guidelines" (which still should be followed). But rule 10 kind of "opens" a possibility of more types, which are not listed in the current config. Should we "remove" it, or let this be as is? In the writer, provided by Benjamin, I see the same types are used as headers in the output changelog. But again, I'm not sure if forcing this set actually "matches the spec".

Is forcing types/scopes/subjects to a specific-case a problem?
Rule 11 basically says "everything, with the exception of BREAKING CHANGE, should be considered case-insensitive. I think this means that writing a type like Feat should be handled like feat. My personal preference is lower case enforcement, but I have no clue if that violates the spec.

Is forcing to forbid full stops in subjects a problem?
Again, I couldn't find this within the spec. An example at rule 5 actually uses this full stop. In HTML, it's rendered as listed below, which kind of includes the . within the <em />.

<em>fix: array parsing issue when multiple spaces were contained in string.</em>

@byCedric
Copy link
Member

byCedric commented Oct 2, 2019

I'll make some time available this weekend to finish the new configuration and support for !. 馃槃

@bcoe
Copy link
Member Author

bcoe commented Oct 3, 2019

awesome thank you @byCedric 馃憤

@byCedric
Copy link
Member

byCedric commented Oct 22, 2019

It took some effort, and a lot of sweat/tears, but it's there. The "next" (now 8.3.3) commitlint version now supports the new conventionalcommits parser, and it's included by default when using @commitlint/config-conventional@next (now 8.3.3). Try it out, let us know what you think or encounter 馃槃馃殌

$ npm i --save @commitlint/cli@next @commitlint/config-conventional@next

I'll try to push for the full spec update because some other developers are reporting conflicting types or casing issues. But that might take a bit longer and probably requires a major bump. Rest assured, we are on it!

@escapedcat
Copy link
Member

Fixed via #821 I believe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

4 participants