concord-bft
concord-bft copied to clipboard
vmware
ci(commitlint): introduce commitlint
The commit introduces commitlint-github-action[1] enforcing enforcing conventional commits[2].
Rationale: Since Concord-BFT is an open-source project, the commit message has to be well-formatted and self-descriptive. The commit message should be structured as follows:
<type>[optional scope]: <description>
[optional body]
[optional footer(s)]
The linter is to be run on pushes and pull requests and will be required. In case the linter fails, it outputs an explanation and a link to the set of rules. The set of rules is defined in commitlint.config.js and based on conventional commits[3]. For more information about Conventional Commits please refer to the official web site[4].
[1] - https://github.com/wagoid/commitlint-github-action [2] - https://conventionalcommits.org/ [3] - https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#rules [4] - https://www.conventionalcommits.org/en/v1.0.0/
Just to be on the same page - your commit creates a linter which requires each commit to have a type (fix, feat, etc) and limits the line length to 120 characters. Is that correct?
Just to be on the same page - your commit creates a linter which requires each commit to have a type (fix, feat, etc) and limits the line length to 120 characters. Is that correct?
It creates a workflow that runs commitlinter which ensures that these conditions are met. I've changed only the maximum line length for commit message body length from 100 to 120.
I see. My opinion is that each contributor should be responsible to write well his commit messages. If he doesn't the PR won't be approved. Enforcing such rules usually causes more frustration than added value, but I believe we can at least try and see if it works for us or not. I'll approve but let's wait for more opinions before merging this.
I'm of the opinion that this won't actually fix our problems with commit messages. The problem is not really one of formatting, but of content. In many cases people don't take the time to document why a change was made, but instead list a few things the change contains. Many times, even that isn't done. Going through the git log will show clearly which commit messages were written with care and thought for the reviewer, and which were done in haste with little long term consideration. The good messages didn't require a specific format, they required valuing the commit history of the project and having empathy for future reviewers and archeologists.
The only formatting change I'd like to see is people stick to the existing guidelines that have been in place for 2 years: https://github.com/vmware/concord-bft/blob/master/CONTRIBUTING.md#formatting-commit-messages
Specifically, Make sure that the first line is under 50 characters, and put a space before the next line. That way the summary is completely visible in the github UI and in git log --oneline. There are plugins for all editors to do this for you, and I'd be fine with a linter for this as well.
On top of that, we need to start rejecting PRs with poorly written messages. I suggest reading and internalizing this document: https://chris.beams.io/posts/git-commit/ That post will do more towards generating quality commit messages than a specific format, which may or may not be appropriate for a given commit.
@f-squirrel Thanks for proposing this change. I believe it should be mandatory for our project to follow the rules outlined in @andrewjstone's comment. What matters most is the semantic of a commit message/header and by enforcing the syntax we can remind contributors to think about proper commit messages. However, in the worst case, it could be well structured garbage. If we cannot have full automation for checking commit messages then how much value do we get from partial automation? I'm on the fence with this PR, I think it is a good idea but doesn't solve the issue we are facing. The change forces commit authors to think about commit messages more but this effect can wear off over time. It would nice to have a tool that checks the what and why (rule 7). It is in the best interest of the commit author and reviewers to write and read well crafted commit messages. So, here is the link again https://chris.beams.io/posts/git-commit/
