Repository navigation
feat(check): Add --allow-abort option - #512
Conversation
8199666 to
cd8c895
Compare
Codecov Report
@@ Coverage Diff @@
## master #512 +/- ##
=======================================
Coverage 97.92% 97.92%
=======================================
Files 39 39
Lines 1540 1543 +3
=======================================
+ Hits 1508 1511 +3
Misses 32 32
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
LGTM but |
Explicitly mark some loop variables unused as recommended by flake8-bugbear.
Lee-W
left a comment
Lee-W
left a comment
There was a problem hiding this comment.
Hi @Kurt-von-Laven , sorry for late reviewing. Your work is fantastic! Just left a few questions. But I think we're almost ready to mrege
Kurt-von-Laven
changed the title
--allow-abort option
cd8c895 to
3c3e9ae
Compare
|
I am not able to reproduce the failure on Python 3.10 locally on Python 3.10.4. It appears to be unrelated to this pull request, so I think the build may simply need to be re-run. Please let me know if anyone sees a connection I am missing. |
|
hmmm. we might need to take a deeper look at the 3.10 case |
Empty commit messages indicate to Git that the commit should be aborted. Displaying an error message when the commit is already being aborted typically only creates confusion. Add an --allow-abort argument to the check command and allow_abort config variable, both defaulting to false for backwards compatibility. When the commit message is empty, determine the outcome based on the allow_abort setting alone, ignoring the pattern.
3c3e9ae to
aa25391
Compare
|
Looks like the Python 3.10 build indeed simply needed to be re-run since all I changed was the two comments. |
3 participants
Description
Empty commit messages indicate to Git that the commit should be aborted. Displaying an error message when the commit is already being aborted typically only creates confusion. Add an
--allow-abortargument to the check command andallow_abortconfig variable, both defaulting to false for backwards compatibility. When the commit message is empty, determine the outcome based on theallow_abortsetting alone, ignoring the pattern.Closes #424.
Checklist
./scripts/formatand./scripts/testlocally to ensure this change passes linter check and testExpected behavior
No error message is displayed when aborting a commit locally, unless the commit .
Steps to Test This Pull Request
cz commit.allow_abort = trueto your config, but an error occurs otherwise.Additional context
Would it make sense for
--allow-abortto also be added as an arg to thecommitcommand? How do folks feel about the argument/setting name and overall approach? See also #247 for a related feature proposal. Since the time of that discussion, I realized that naming a Commitizen argument--allow-empty-messagecould prove extremely confusing, because passing it to Commitizen would not, by itself, cause commits with empty messages to be committed like the Git argument of the same name. It would also interact poorly with the proposal to allow ferrying arbitrary arguments togit commitin #451, because it would be easy to accidentally pass the argument to Git, resulting in commits with empty messages no longer being aborted.