search

conventional-changelog-archived-repos / validate-commit-msg

DEPRECATED. Use https://github.com/marionebl/commitlint instead. githook to validate commit messages are up to standard
http://conventionalcommits.org/
MIT License
556 stars 101 forks source link

autoFix does not fix #105

Closed cmalard closed 6 years ago

cmalard commented 6 years ago

Hello guys,

Not really sure to understand autoFix:

If this is set to true, type will be auto fixed to all lowercase, subject first letter will be lowercased, and the commit will pass (assuming there's nothing else wrong with it).

Actually it is not renaming the commit. And I do not see anything to re-write the COMMIT_MSG. Did I missed something ? @spirosikmd Sorry if this is the case, I would love to use that ;-)

Garbee commented 6 years ago

I'll take a look at this after an interview in a bit. The code appears to be in place. But I'm not sure how we'd auto-update a commit message on-the-fly before git gets it back.

spirosikmd commented 6 years ago

Hi @cmalard! I think naming is a bit confusing here indeed. It does not auto fix the commit message. The code in place temporarily adjusts the commit message (type and subject), to not fail the validation. Basically this option should probably be renamed to ignoreCase? Something like this! What do you think @Garbee?

Garbee commented 6 years ago

Renaming doesn't really do much until v2 when we can remove the current config option name. For now a quick documentation update is the best path forward.

I'm not even seeing a great use-case for ignoreCase unless your software consuming the structure is adjusting things as well. Otherwise you could end up with a separate button and Button section of update docs when really they are the same.

It feels to me like not having an option to ignore casing would be the best way. Be as strict as possible about things. If developers want to not care about casing, they can apply all the casing they want to allow in their config. This keeps the entire log history as consistent as possible.

spirosikmd commented 6 years ago

I agree with being strict and consistent. Maybe actually auto-fixing the commit message would be an option. Instead of just ignoring case during validation, which sounds fragile. The purpose of autoFix was to fix #37. But now that I am going through it, we should have better actually auto-fix the commit message. I am not sure if this should be a feature for v2.

Garbee commented 6 years ago

Looks like editing it in-place should be possible. So the current expected functionality can be fixed.

Ref: https://git-scm.com/docs/githooks#_commit_msg

The hook is allowed to edit the message file in place, and can be used to normalize the message into some project standard format. It can also be used to refuse the commit after inspecting the message file.

If someone wants to PR this I can review it once it lands. Otherwise it'll be Sunday or Monday before I can work on it myself.