fix(pre-commit): Correct pre-commit Hook Definition #514
Conversation
require_serial is not pertinent to commit-msg hooks or hooks that themselves execute serially. The option is intended to prevent fork-bombing that can occur when pre-commit runs a hook that itself spawns many processes many times in parallel.
pre-commit 1.4.3, released 2018-01-02, is the minimum pre-commit version at which language_version: python3 is translated to the correct py launcher call on Windows.
The Commitizen pre-commit hook runs cz check, which only makes sense to do on a commit message, not at other Git hook stages. By default the hook is currently run on staged files pre-commit, which is nonsensical unless the files in the repository themselves contain Git commit messages. The hook is an implausible candidate even for the prepare-commit-msg stage since this stage is intended for hooks that modify the commit message rather than check it. Specifying the hook stage centrally makes it less error-prone to use and saves one line of configuration for users of the pre-commit hook.
Prevent pre-commit hook from complaining when a commit is aborted by default, but allow users to override this option in their pre-commit config by specifying it in args, not entry. Move --commit-msg-file option from entry to args since it has to be the last option.
Codecov Report
@@ Coverage Diff @@
## master #514 +/- ##
==========================================
- Coverage 97.92% 97.86% -0.07%
==========================================
Files 39 39
Lines 1540 1543 +3
==========================================
+ Hits 1508 1510 +2
- Misses 32 33 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Please let me know what action I should take regarding the Codecov report. My instinct is to consider it a false positive and ignore it since I believe it is a consequence of increasing the length of the pre-commit hook definition length by one net line. Although, maybe the ideal thing would be to configure Codecov not to scan |
|
I just browse through the code. Everything looks good, but I might a bit more time to test it out. I think we'll be able to merge it this week. Thank you so much for your active contributions 🙏
I'm good with ignore the warning here. |
|
just tested. let's merge it 🎉 |
Description
Use new
--allow-abortoption. Prevent pre-commit hook from complaining when a commit is aborted by default, but allow users to override this option in their pre-commit config by specifying it inargs, notentry. Move--commit-msg-fileoption fromentrytoargssince it has to be the last option.Confine hook to
commit-msgstage. The Commitizen pre-commit hook runscz check, which only makes sense to do on a commit message, not at other Git hook stages. By default the hook is currently run on staged filespre-commit, which is nonsensical unless the files in the repository themselves contain Git commit messages. The hook is an implausible candidate even for theprepare-commit-msgstage since this stage is intended for hooks that modify the commit message rather than check it. Specifying the hook stage centrally makes it less error-prone to use and saves one line of configuration for users of the pre-commit hook.Change minimum pre-commit version from 0.15.4 to 1.4.3. pre-commit 1.4.3, released 2018-01-02, is the minimum pre-commit version at which
language_version: python3is translated to the correct py launcher call on Windows.Don't require serial execution.
require_serialis not pertinent tocommit-msghooks or hooks that themselves execute serially. The option is intended to prevent fork-bombing that can occur when pre-commit runs a hook that itself spawns many processes many times in parallel.Checklist
./scripts/formatand./scripts/testlocally to ensure this change passes linter check and testExpected behavior
The commit hook continues to work as before but without requiring that the
commit-msgstage be specified in your.pre-commit-config.yamlor giving a misleading error message when the commit is aborted by saving an empty commit message in your editor.Steps to Test This Pull Request
Replace your Commitizen hook config in your
.pre-commit-config.yamlwith the following:Ensure that you have pre-commit 1.4.3 or higher installed.
Install
commit-msghooks if you haven't already via:pre-commit install --hook-type commit-msg.Commit some changes to your repository.
If the commit message was invalid, the hook will fail, and otherwise it will succeed. Empty commit messages now fall in the latter category, and the commit aborts cleanly without a misleading error.
Additional context
Follows on #512, which added the
--allow-abortoption tocz check. Reverts #135, which removedstages: [commit-msg].