commitizen icon indicating copy to clipboard operation
commitizen copied to clipboard

feat(pre-commit): Add commitizen-branch hook

Open Kurt-von-Laven opened this issue 3 years ago • 1 comments

Description

Check all commit messages on the current branch. This is useful for checking commit messages after the fact (e.g., pre-push or in CI) since the existing hook only works at commit time. Expand the documentation of the pre-existing commitizen hook to clarify the relationship between them. I wasn't sure whether it would be appropriate to add a section to this documentation.

Remove no longer needed commit-msg stage from self-use of commitizen pre-commit hook in .pre-commit-config.yaml. In v2.27.1, the commitizen pre-commit hook began specifying that it runs on the commit-msg stage in the hook definition in .pre-commit-hooks.yaml, so it is no longer necessary to specify the hook stage when using the hook in .pre-commit-config.yaml.

Checklist

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

Expected behavior

The commitizen hook continues to work as before, allowing empty commit messages. The commitizen-branch hook checks all commit messages on your branch, and doesn't allow empty commit messages.

Steps to Test This Pull Request

  1. Replace your Commitizen hook config in your .pre-commit-config.yaml with the following:

    repos:
      - repo: https://github.com/Kurt-von-Laven/commitizen
        rev: pre-commit-hook
        hooks:
          - id: commitizen
          - id: commitizen-branch
            stages: [push]
    
  2. Ensure that you have pre-commit 1.4.3 or higher installed.

  3. Install commit-msg and pre-push hooks if you haven't already via: pre-commit install --hook-type commit-msg pre-push.

  4. Commit some changes to your repository.

  5. If the commit message was invalid, the commitizen hook will fail, and otherwise it will succeed.

  6. Commit a change with an empty commit message: git commit --allow-empty-message -m "".

  7. Attempt to push your branch.

  8. The push fails with an error regarding the empty commit message.

Additional context

Follows on #512 and #514.

Kurt-von-Laven avatar May 22 '22 23:05 Kurt-von-Laven

Codecov Report

Merging #517 (01ce73b) into master (764861f) will decrease coverage by 0.32%. The diff coverage is n/a.

:exclamation: Current head 01ce73b differs from pull request most recent head bd4aa6f. Consider uploading reports for the commit bd4aa6f to get more accurate results

@@            Coverage Diff             @@
##           master     #517      +/-   ##
==========================================
- Coverage   98.25%   97.93%   -0.33%     
==========================================
  Files          39       39              
  Lines        1551     1551              
==========================================
- Hits         1524     1519       -5     
- Misses         27       32       +5     
Flag Coverage Δ
unittests 97.93% <ø> (-0.33%) :arrow_down:

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

Impacted Files Coverage Δ
commitizen/changelog.py 96.72% <0.00%> (-2.72%) :arrow_down:
commitizen/cli.py 93.93% <0.00%> (ø)
commitizen/cmd.py 100.00% <0.00%> (ø)
commitizen/git.py 100.00% <0.00%> (ø)
commitizen/commands/bump.py 96.42% <0.00%> (ø)
commitizen/commands/check.py 100.00% <0.00%> (ø)
...en/cz/conventional_commits/conventional_commits.py 100.00% <0.00%> (ø)

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

codecov[bot] avatar May 22 '22 23:05 codecov[bot]

Users can certainly write their own pre-commit hooks, but I consider them tricky to write and test since many implementations I see that haven't already been reviewed by the author of pre-commit get details wrong. I also believe many users will not be aware that the commit-msg hook relies on the developer to have installed the pre-commit hooks correctly since it can't be run in CI. It is a good hook in that it offers real-time feedback, but it is also trivially circumvented (most likely by accident). Tools like pre-commit.ci, pre-commit/pre-commit-action, and ScribeMD/pre-commit-action make it trivial to run the proposed pre-push hook in CI, which is not possible with the existing commit-msg hook.

Kurt-von-Laven avatar Aug 16 '22 23:08 Kurt-von-Laven

The test failures seem unrelated to this pull request.

Kurt-von-Laven avatar Aug 17 '22 02:08 Kurt-von-Laven