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

Replace commitlint with a more generic and less fragile solution #6668

Open
sschuberth opened this issue Mar 13, 2023 · 14 comments
Open

Replace commitlint with a more generic and less fragile solution #6668

sschuberth opened this issue Mar 13, 2023 · 14 comments
Labels
configuration About configuration topics release On the topic of making releases

Comments

@sschuberth
Copy link
Member

sschuberth commented Mar 13, 2023

Since the introduction of conventional commits and commitlint to enforce them we've faced various issues with commitlint, resulting in work-arounds like

ort/.commitlintrc.yml

Lines 5 to 11 in 2ce4322

parserOpts:
headerPattern: '^(\w*)(?:\((.*)\))?!?: (.*)$'
breakingHeaderPattern: '^(\w*)(?:\((.*)\))?!: (.*)$'
headerCorrespondence: ['type', 'scope', 'subject']
noteKeywords: ['BREAKING CHANGE', 'BREAKING-CHANGE', '\[\d+\]:', 'Signed-off-by:']
revertPattern: '/^(?:Revert|revert:)\s"?([\s\S]+?)"?\s*This reverts commit (\w*)\./i'
revertCorrespondence: ['header', 'hash']

Also the configuration syntax sucks quite frankly, like in

ort/.commitlintrc.yml

Lines 13 to 15 in 727fc5b

body-leading-blank:
- 2
- always

what the heck is "2" supposed to mean? Let alone the bugs.

As conventional commit can easily be checked via regexes, I propose to instead switch to a generic tool like https://github.com/GsActions/commit-message-checker that can also be configured to check for some of our other non-conventional-commits requirements.

Opinions, @oss-review-toolkit/core-devs?

Edit: Collecting ideas for alternatives

@sschuberth sschuberth added release On the topic of making releases configuration About configuration topics labels Mar 13, 2023
@MarcelBochtler
Copy link
Member

While I get the issues with commitlint, continue using it has some advantages:

  • I can use the commitlint CLI to create a post commit hook.
  • I can use editor plugins (e.g. for vim / IntelliJ) that already verify the format of the commit message.

My vote is to continue using commitlint.

@sschuberth
Copy link
Member Author

sschuberth commented Mar 13, 2023

I can use editor plugins (e.g. for vim / IntelliJ) that already verify the format of the commit message.

I'm not sure I can follow here. How's that related to commitlint specifically vs. any other conventional commits checker?

@sschuberth
Copy link
Member Author

sschuberth commented Mar 13, 2023

I can use the commitlint CLI to create a post commit hook.

Ok, if it's about CLI support, how about using https://github.com/cocogitto/cocogitto instead? That's at least written in Rust instead of TypeScript... 😉 Its accompanying https://github.com/cocogitto/cocogitto-bot also comments about failures instead of hiding the cause in logs.

@mnonnenmacher
Copy link
Member

I'm open to alternatives, but they must be as easy to run locally as commitlint (e.g. npx commitlint --from HEAD~2). Ideally, it should be possible to integrate it with Gradle to not depend on more tools (like currently npx).

@MarcelBochtler
Copy link
Member

MarcelBochtler commented Mar 13, 2023

I can use editor plugins (e.g. for vim / IntelliJ) that already verify the format of the commit message.

I'm not sure I can follow here. How's that related to commitlint specifically vs. any other conventional commits checker?

I'm mostly talking about CLI tools, but the commitlint ecosystem also offers plugins for multiple editors:
For instance IntelliJ or VSCode, and they might even be smart enough to use the .commitlintrc.yml to apply the repositories commitlint settings.

But more important for me is that a CLI tool exists that can use something like the .commitlintrc.yml to ensure locally that the last commit is correct and complies with the repositories commitlint settings.

As conventional commit can easily be checked via regexes, I propose to instead switch to a generic tool like https://github.com/GsActions/commit-message-checker that can also be configured to check for some of our other non-conventional-commits requirements.

IIUC this would rely on regexes written in the CI definition files, which couldn't be easily reused by any local tools.

@sschuberth
Copy link
Member Author

Ideally, it should be possible to integrate it with Gradle to not depend on more tools (like currently npx).

Maybe https://github.com/nicolasfara/conventional-commits?

@mnonnenmacher
Copy link
Member

The plugin does not seem to be very configurable.
To me we could just close this issue as I hadn't had any issues with commitlint for a while and am using it in multiple repositories now.

@sschuberth
Copy link
Member Author

The plugin does not seem to be very configurable.

What other configuration beyond the available options would you expected from a tool that is specialized on conventional commits (not commit syntax in general)?

To me we could just close this issue as I hadn't had any issues with commitlint for a while

Once you configure around all the quirks in commitlint it somewhat works, yes. But my point is that there are too many quirks. IMO it's a badly written tool. So I'd like to keep the issue open for a while more.

@mnonnenmacher
Copy link
Member

What other configuration beyond the available options would you expected from a tool that is specialized on conventional commits (not commit syntax in general)?

I would like the tool to verify not just the conventional commit types and scopes but also the whole commit syntax indeed. So line length, casing, and so on. Also, it seems the linked Gradle plugin does only work as a commit hook, I would not like that.

@sschuberth
Copy link
Member Author

sschuberth commented Oct 29, 2023

hadn't had any issues with commitlint for a while

Here's again a bogus error I'm running into. I'd really still like to get rid of commitlint.

@mnonnenmacher
Copy link
Member

Here's again bogus error I'm running into.

Issue references are part of the footer so it's complaining because there is no empty line before "Fixes #7366". The error message is not helpful though.

@sschuberth
Copy link
Member Author

Issue references are part of the footer

What? No, why? Only my Signed-off-by is a footer line. I believe this is simply conventional-changelog/commitlint#896.

@sschuberth
Copy link
Member Author

Issue references are part of the footer

What? No, why? Only my Signed-off-by is a footer line. I believe this is simply conventional-changelog/commitlint#896.

Ah, you might be confused by yet another bug which adds blank lines in the error output where there are none: conventional-changelog/commitlint#3129

@mnonnenmacher
Copy link
Member

Commitlint defaults to the conventional-commits-parser: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/parse#api

And this interprets issue references to be part of the footer: https://github.com/conventional-changelog/conventional-changelog/blob/master/packages/conventional-commits-parser/README.md#usage

The issue will go away if you add an empty line before "Fixes #7366" (like we usually format it anyway).

Ah, you might be confused by yet another bug which adds blank lines in the error output where there are none: conventional-changelog/commitlint#3129

This bug makes it worse because the actual issue is not visible in the error output because of it.

IMO it's a badly written tool.

I do agree with that, it seems to be overly complex for the problem it's trying to solve and the documentation is terrible.

I'd really still like to get rid of commitlint.

If there was an alternative that fulfills the requirements mentioned in the messages above I would be happy to switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration About configuration topics release On the topic of making releases
Projects
None yet
Development

No branches or pull requests

3 participants