ci: Add a conv. commit lint action+config#5598
Conversation
| @@ -0,0 +1,20 @@ | |||
| module.exports = { | |||
| extends: ['@commitlint/config-conventional'], | |||
| rules: { | |||
There was a problem hiding this comment.
Why these particular rules? What does the [0] do? Can we just use someone else's config? Like what if we didn't "extend" the config-conventional
There was a problem hiding this comment.
We already add this to s2n-quic repo: aws/s2n-quic#2881
The doc for this rule can be found in https://commitlint.js.org/reference/rules-configuration.html#rules-configuration.
There was a problem hiding this comment.
It seemed like a reasonable starting point... here is the complete list
@commitlint/config-angular
@commitlint/config-conventional
@commitlint/config-lerna-scopes
@commitlint/config-nx-scopes
@commitlint/config-patternplate
@commitlint/config-workspace-scopes
conventional-changelog-lint-config-atom
conventional-changelog-lint-config-canonical
For subject-case 0 turns this off; I could not sus out a common pattern to past commit subject case; that isn't to say we shouldn't pick one, but I'm trying to ease us into this check. I can also add more doc links if that would help?
There was a problem hiding this comment.
Hmm, it seems like the most reasonable starting point is just their default? I understand that it might cause a bit of friction, but one of the benefits of this tooling is the consistency. If we hate it we can always change it 🤷
Some of the ones that we've specified seem less than ideal e.g. allowing test and tests.
There was a problem hiding this comment.
Can you add this link https://commitlint.js.org/reference/rules-configuration.html#rules-configuration to this file?
| @@ -0,0 +1,20 @@ | |||
| module.exports = { | |||
| extends: ['@commitlint/config-conventional'], | |||
| rules: { | |||
There was a problem hiding this comment.
Can you add this link https://commitlint.js.org/reference/rules-configuration.html#rules-configuration to this file?
| @@ -0,0 +1,20 @@ | |||
| module.exports = { | |||
| extends: ['@commitlint/config-conventional'], | |||
| rules: { | |||
There was a problem hiding this comment.
Hmm, it seems like the most reasonable starting point is just their default? I understand that it might cause a bit of friction, but one of the benefits of this tooling is the consistency. If we hate it we can always change it 🤷
Some of the ones that we've specified seem less than ideal e.g. allowing test and tests.
| issuePrefixes: ['#'] | ||
| } | ||
| }, | ||
| ignores: [ |
There was a problem hiding this comment.
Why do we need to ignore this? And what is actually getting ignored? Like conventional commits aren't enforced if the commit includes "Co-authored-by"?
There was a problem hiding this comment.
It's a line by line check, from my local testing. If someone adds a "recommended change" to your PR and you accept it, Co-authored-by gets added to the commit message. With-out this present, you would need to rebase and strip these out for this check to pass; example.
There was a problem hiding this comment.
Hmm, why does it fail that commit?
Co-authored-by: maddeleine <59030281+maddeleine@users.noreply.github.com>
| rules: { | ||
| 'subject-case': [0], | ||
| 'body-max-line-length': [0], | ||
| 'footer-max-line-length': [0], | ||
| 'type-enum': [2, 'always', [ | ||
| 'build', 'chore', 'ci', 'docs', 'feat', 'fix', 'perf', 'test' | ||
| ]] |
There was a problem hiding this comment.
| rules: { | |
| 'subject-case': [0], | |
| 'body-max-line-length': [0], | |
| 'footer-max-line-length': [0], | |
| 'type-enum': [2, 'always', [ | |
| 'build', 'chore', 'ci', 'docs', 'feat', 'fix', 'perf', 'test' | |
| ]] |
Maybe I'm missing something, but can we just delete these and totally take everything from the predefined stuff?
There was a problem hiding this comment.
Subject case is pretty pedantic; I'm in favor of strictness, but I'm trying to ease into this change and avoid the inevitable "why doesn't this pass?" question by relaxing it a bit.
|
Reworking in favor of a PR title checker. |
Release Summary:
Resolved issues:
resolves none
Description of changes:
Adds a GitHub Action to check the last commit for compliance with conventional commits.
Call-outs:
Shooting for parity with the configuration in https://github.com/aws/s2n-quic.
Testing:
In CI this run should pass...as this PR commit is using conventional commits.
Locally, we can go back in time and find a commit that didn't follow the format:
If there are no issues found, the command exits with no output.
The issues were:
refactor 1/2:isn't quite right, if the 1/2 had been in()this would have passed...bindings():is not a valid type.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.