-
-
Notifications
You must be signed in to change notification settings - Fork 281
feat: add an argument to limit the length of commit message #1076
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
feat: add an argument to limit the length of commit message #1076
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1076 +/- ##
==========================================
+ Coverage 97.33% 97.58% +0.24%
==========================================
Files 42 55 +13
Lines 2104 2486 +382
==========================================
+ Hits 2048 2426 +378
- Misses 56 60 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @kevin1kevin1k ! Left a few nitpicks. But we're pretty close to merge this one
commitizen/commands/commit.py
Outdated
return cz.message(answers) | ||
|
||
message = cz.message(answers) | ||
message_len = len(message.partition("\n")[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know we could use partition
👀 Great job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we strip()
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, thanks for the suggestion!
Also, great work on the detailed PR description. |
Great job @kevin1kevin1k ! @woile @noirbizarre I'm planing to merge this one these days. Let me know if you want to take a deeper look. Thanks! |
Description
Re-opened another PR for #191 as the long-hanging #557 has been closed.
Check if the length of commit message exceeds the specified limit.
The limit can be specified via, for example,
-l 72
or--message-length-limit 72
.Checklist
./scripts/format
and./scripts/test
locally to ensure this change passes linter check and testExpected behavior
If one runs
cz commit -l/--message-length-limit N
, the commit would fail (with the exceptionCommitMessageLengthExceededError
) when the message is longer thanN
characters.Note that for
ConventionalCommitsCz
, the limit applies only from the prefix to the subject.In other words, everything after the first line (the body and the footer) are not counted in the length.
Steps to Test This Pull Request
poetry run python commitizen/cli.py -n cz_conventional_commits c -l 10
(orcz -n cz_conventional_commits c -l 10
when merged)fix:
with length 5)123456
with length 6As 5+6 = 11 > 10 the process should exit with message
Length of commit message exceeds limit (11/10)
and error code 32.Additional context
Closes: #191