conventional-changelog / commitlint

📓 Lint commit messages
https://commitlint.js.org
MIT License
16.36k stars 883 forks source link

@commitlint/config-conventional rejects default merge commit messages #365

Closed jedwards1211 closed 5 years ago

jedwards1211 commented 6 years ago

I'm surprised no one has run across this, but I think it only happens after there are merge conflicts, because if there are no conflicts in the merge I think git automatically commits without running the commit message hook.

Expected Behavior

When I have merge conflicts, fix them, and then try to commit, commitlint should accept the auto-generated merge commit message.

Current Behavior

commitlint rejects the auto-generated merge commit message:

⧗   input: Merge branch 'master' of https://github.com/jcoreio/webapp-apollo
✖   message may not be empty [subject-empty]
✖   type may not be empty [type-empty]
✖   found 2 problems, 0 warnings

Affected packages

Possible Solution

Steps to Reproduce (for bugs)

  1. Edit any file in a project using @commitlint/config-conventional
  2. Try to commit with a fake merge commit message: git commit -m "Merge branch 'master' of https://github.com/commitlint/cli"

Context

Your Environment

Executable Version
commitlint --version 5.2.8
git --version 2.15.1 (Apple Git-101)
node --version v8.9.2
byCedric commented 6 years ago

Currently only Merged is listed in the ignore list, you can see all patterns over here. May I ask what Git provider you are using which uses the Merge notation instead of Merged? Maybe it's a good idea to add the Merge version too.

jedwards1211 commented 6 years ago

I'm a little confused, I'm using the git cli on macOS. This merge was a result of git pull skeleton master, maybe some git commands cause different commit wording?

byCedric commented 6 years ago

@jedwards1211 My bad, I didn't mean to confuse you! I mistook this bug report as a feature request. As far as I can tell, you are using an old version of commitlint. In the latest version (7.0.0) this message is ignored by the @commitlint/is-ignored package. Could you try upgrading to this version?

marionebl commented 5 years ago

Thanks for reporting @jedwards1211 and triage @byCedric! Closing this as I am very confident this is fixed in recent commitlint versions as mentioned by @byCedric here: https://github.com/marionebl/commitlint/issues/365#issuecomment-403802596

dabit1gamma commented 5 years ago

Guys it is still not working. The regexp is not correct..

byCedric commented 5 years ago

@dabit1gamma could you provide some examples where the regular expression fails for you? I think we have modified it a bit more in #439. But if you could provide some cases where the current expression is not working for you, that will be helpful. 😄

dabit1gamma commented 5 years ago

@byCedric I created a PR. With the modifications of #439 it is still not working. You can see an example which does not work in my PR. Let's discuss it :)

My PR: #454

Zara603 commented 1 year ago

still not working 2023

Zara603 commented 1 year ago

after merge conflict Screenshot 2023-05-08 at 11 58 49 am

escapedcat commented 1 year ago

after merge conflict Screenshot 2023-05-08 at 11 58 49 am

I think this issue is about generated commit messages like Merge branch 'master' of....
In your you could just use this I think: chore: resolved merge conflict

mebibou commented 1 month ago

How can we disable this behaviour? I looked at the documentation but I can't really see what I'm supposed to change to prevent all "Merge branch..." commits

My current config is:

module.exports = {extends: ['@commitlint/config-conventional']}
escapedcat commented 1 month ago

How can we disable this behaviour? I looked at the documentation but I can't really see what I'm supposed to change to prevent all "Merge branch..." commits

True, the docs could be improved. Happy for a PR.

Have a look at https://commitlint.js.org/reference/configuration.html#configuration-object-example You can set defaultIgnores: false.

mebibou commented 1 month ago

Ah yes thanks, could not find this particular page. The name of the key is not very clear imo, maybe useDefaultIgnores would have been better

mebibou commented 1 month ago

Another idea could be to have different presets depending on the merge method. When using rebase strategy, these commit messages should not be allowed