Skip to content

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

Closed

Description

@Kurt-von-Laven

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

Activity

  1. Lee-W commented on Sep 26, 2021

    @Lee-W
    Member

    @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.

  2. Kurt-von-Laven commented on Sep 26, 2021

    @Kurt-von-Laven
    ContributorAuthor

    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.

  3. Lee-W commented on Sep 26, 2021

    @Lee-W
    Member

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

  4. Lee-W commented on Nov 26, 2021

    @Lee-W
    Member

    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions