ljnsn / cz-conventional-gitmoji

A commitizen plugin that combines gitmoji and conventional commits.
13 stars 1 forks source link

Gitmojify pre-commit hook fails for merge commits that commitizen allows #166

Open kenibrewer opened 1 month ago

kenibrewer commented 1 month ago

When merging changes from a main branch into feature branch as follows:

git checkout feature-branch
git merge main
> Auto-merging CHANGELOG.md
> CONFLICT (content): Merge conflict in CHANGELOG.md
> Automatic merge failed; fix conflicts and then commit the result.
git add .
git commit
< close text editor with default "Merge branch 'main' into feature-branch" >

This error occurs:

gitmojify................................................................Failed
- hook id: conventional-gitmoji
- exit code: 1

Traceback (most recent call last):
  File "/home/vscode/.cache/pre-commit/repo75cf19di/py_env-python3/bin/gitmojify", line 8, in <module>
    sys.exit(run())
             ^^^^^
  File "/home/vscode/.cache/pre-commit/repo75cf19di/py_env-python3/lib/python3.11/site-packages/gitmojify/mojify.py", line 57, in run
    f.write(gitmojify(msg))
            ^^^^^^^^^^^^^^
  File "/home/vscode/.cache/pre-commit/repo75cf19di/py_env-python3/lib/python3.11/site-packages/gitmojify/mojify.py", line 42, in gitmojify
    raise ValueError("invalid commit message")
ValueError: invalid commit message

This behavior is different from commitizen which includes "Merge" commits in its list of ignored commits. See docs.

One possible solution would be fetching the set of allowed commits messages from the commitizen configuration and allowing them. That seems pretty complicated though.

The approach that seems both a bit easier to implement and potentially more on theme with gitmojify would be automatically correct these non-conventional commits into gitmojified commits. The one downside to that approach is that would potentially mean that these low-information commits might get added to the changelog when previously they weren't.

ljnsn commented 1 month ago

Mmm interesting, the allowed_prefixes are the default ones, but they are indeed not even read by gitmojify. I usually set up gitlab/github to use a gitmojified commit message for merges and I rarely ever merge locally, so haven't come across this.

One possible solution would be fetching the set of allowed commits messages from the commitizen configuration and allowing them.

I think this is what should happen. At some point this package should provide some configuration options too, so then we would need to hook into the commitizen config in any case.

The approach that seems both a bit easier to implement and potentially more on theme with gitmojify would be automatically correct these non-conventional commits into gitmojified commits.

I'm not a huge fan of that tbh, right now gitmojify just adds the corresponding emoji and doesn't alter the message in any other way, auto converting the message itself to be conventional compliant is not really in scope for this lib.

When I find some time I'll look into your first suggestion. In the meantime, PRs are always welcome! :)