Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Aborted Commit Considered to Fail Validation When Run As commit-msg Hook #424

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

Closed
Kurt-von-Laven opened this issue Sep 17, 2021 · 4 comments · Fixed by #512
Closed

Aborted Commit Considered to Fail Validation When Run As commit-msg Hook #424

Kurt-von-Laven opened this issue Sep 17, 2021 · 4 comments · Fixed by #512
Labels
type: feature A new enhacement proposal

Comments

@Kurt-von-Laven
Copy link
Contributor

Kurt-von-Laven commented Sep 17, 2021

Description

Git interprets an empty commit message to mean that a commit should be aborted, but calls all commit-msg hooks anyways. This causes pre-commit hooks that run as commit-msg hooks but don't guard against empty commit messages to spew errors en masse when the commit was already being intentionally aborted.

Steps to reproduce

  1. Configure commitizen to run as a commit-msg hook using pre-commit as described below.
  2. touch foo && git add && git commit
  3. Save an empty commit message.

For step 1, add the following to .pre-commit-config.yaml:

yaml
  - repo: https://github.com/commitizen-tools/commitizen
    rev: v2.18.1
    hooks:
      - id: commitizen
        stages:
          - commit-msg

Current behavior

Pre-commit follows git's lead and calls commitizen when the commit message is empty when run as a commit-msg hook. This leads to the following error:

commitizen check..............................................................Failed
- hook id: commitizen
- exit code: 14

commit validation: failed!
please enter a commit message in the commitizen format.
commit "": "# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch foo
#
# Changes to be committed:
#       ,,,
pattern: (build|ci|docs|feat|fix|perf|refactor|style|test|chore|revert|bump)(\(\S+\))?!?:(\s.*)

The commit is still correctly aborted of course.

Desired behavior

I expected the commitizen commit-msg hook to silently report success in this case since there is no commit message to check.

Environment

  • commitizen version: 2.18.1
  • python version: Python 3.9.5
  • operating system: Linux
@Lee-W
Copy link
Member

Lee-W commented Sep 26, 2021

@Kurt-von-Laven Hi, thanks for reporting! I'm a bit against this feature. Even though the commit is an empty one, it's still a git commit and should be checked.

@Kurt-von-Laven
Copy link
Contributor Author

Kurt-von-Laven commented Sep 26, 2021

I get where you are coming from. Certainly that is more technically correct to check the commit message no matter what. To be clear, I am referring to empty commit messages as opposed to empty commits. My argument here is that the commit only goes through if the user explicitly requested that an empty commit message be allowed, and empirically I have never actually seen anyone push an empty commit message since typing --allow-empty-message takes longer than typing -m "fixed stuff". It can be convenient to make empty commit messages for commits that are being immediately squashed or dropped locally, for instance. By far the more common case though is that the commit is being aborted, in which case the error output is misleading (particularly for beginners). That being said, I can see it being frustrating to force tolerance for empty commit messages on people, so I wonder if it would make sense to offer an --allow-empty-message flag as Git does.

@Lee-W
Copy link
Member

Lee-W commented Sep 26, 2021

Get your point now. if that's the case, I think it's a similar issue to #247. This feature sounds good to me 👍

@Lee-W Lee-W added type: feature A new enhacement proposal and removed type: bug discussion labels Sep 26, 2021
@Lee-W
Copy link
Member

Lee-W commented Nov 26, 2021

as this issue is simliar to #247 , I'll close this one and let's discuss there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new enhacement proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants