feat: check if commit message is no more than 72 characters #557

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

Jump to bottom

Closed

kevin1kevin1k wants to merge 0 commits into commitizen-tools:master from kevin1kevin1k:feature/check-commit-message-length

Closed

feat: check if commit message is no more than 72 characters #557

kevin1kevin1k wants to merge 0 commits into commitizen-tools:master from kevin1kevin1k:feature/check-commit-message-length

Conversation

kevin1kevin1k

Copy link

Contributor

@kevin1kevin1k kevin1kevin1k commented Aug 14, 2022 •

edited

Loading

Description

Check if the length of commit message (the prefix(scope): subject part) does not exceed 72 characters.
Currently only supports ConventionalCommitsCz; JiraSmartCz and CustomizeCommitsCz not implemented.
An imperfect workaround for #191.

Checklist

Add test cases to all the changes you introduce
Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
Test the changes on the local machine manually
Update the documentation for the changes

Expected behavior

If run cz commit with -cl or --check-length argument, the commit would fail when prefix(scope): subject is longer than 72 chars.
Without the argument, the default behavior is to accept longer messages.

Steps to Test This Pull Request

cz commit -cl
ENTER (type prefix: fix)
ENTER (empty scope)
01234567890123456789012345678901234567890123456789012345678901234567 (for subject)
ENTER (empty body)
ENTER (not breaking change)
ENTER (no footer)

Then cz should exit with message Length of commit message exceeded limit (73/72) and error code 23.

Additional context

@kevin1kevin1k kevin1kevin1k mentioned this pull request

Aug 14, 2022

[Feature Request] Line length warning #191

Closed

@codecov

Copy link

codecov bot commented Aug 14, 2022 •

edited

Loading

Codecov Report

Merging #557 (44c145d) into master (764861f) will decrease coverage by 0.00%.
The diff coverage is 98.63%.

@@ Coverage Diff @@
## master #557 +/- ##
==========================================
- Coverage 98.25% 98.25% -0.01% 
==========================================
 Files 39 39 
 Lines 1551 1604 +53 
==========================================
+ Hits 1524 1576 +52 
- Misses 27 28 +1

Flag	Coverage Δ
unittests	`98.25% <98.63%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
commitizen/cli.py	`93.93% <ø> (ø)`
...en/cz/conventional_commits/conventional_commits.py	`98.59% <88.88%> (-1.41%)`	⬇️
commitizen/__version__.py	`100.00% <100.00%> (ø)`
commitizen/bump.py	`100.00% <100.00%> (ø)`
commitizen/cmd.py	`100.00% <100.00%> (ø)`
commitizen/commands/changelog.py	`96.62% <100.00%> (+0.03%)`	⬆️
commitizen/commands/commit.py	`98.50% <100.00%> (+0.09%)`	⬆️
commitizen/commands/init.py	`91.66% <100.00%> (ø)`
commitizen/config/json_config.py	`100.00% <100.00%> (ø)`
commitizen/config/yaml_config.py	`100.00% <100.00%> (ø)`
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Lee-W Lee-W self-requested a review

August 16, 2022 09:54

@Lee-W Lee-W self-assigned this

Aug 16, 2022

Lee-W

Lee-W reviewed

Aug 20, 2022

View reviewed changes

commitizen/commands/commit.py Outdated

self.config: BaseConfig = config

self.cz = factory.commiter_factory(self.config)

self.arguments = arguments

print(self.config)

Copy link

Member

@Lee-W Lee-W Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest we don't print anything not directly related to cz commit

commitizen/cz/conventional_commits/conventional_commits.py Outdated

message = f"{prefix}{scope}: {subject}{body}{footer}"

message = f"{prefix}{scope}: {subject}"

message_len = len(message)

MESSAGE_LEN_LIMIT = 72

Copy link

Member

@Lee-W Lee-W Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make it configurable?

Copy link

Member

@Lee-W Lee-W Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in the comment #191 (comment), he proposes a reasonable solution. Although global variables might sometimes not be desired. I'm good with it in this case. @kevin1kevin1k @woile What do you think?