Validating commit msg against all commits which are ahead

[svattenky]

Is there a way to validate all the commits which are ahead of a origin branch? My use case is this I don't want to validate on every single commit message as my team does a bunch of micro commits however when they do a push I want to validate all the commit messages that are being pushed. It seems to only do the last one by default

[kentcdodds]

I need to give you an apology. For some reason, I want watching this repository so I want getting notifications about issues and pull requests. I am so incredibly sorry. Please forgive me. I really feel awful. I hope my unresponsiveness doesn't hinder you contributing more in the future.

As for your specific pull request, I'll give it a closer look soon.

[kentcdodds]

You are correct. This repository is specifically used as a commit-msg hook. But perhaps it could be enhanced to separate the commit validation api and the cli behavior. If you'd be interested, I'd happily accept a pull request that separates the cli behavior from the validation function. This would clean up the code quite a bit as well. :+1:

Let me know what you think.

[Garbee]

I'm wondering why this should be this projects concern. In my view, validate-commit-msg simply validates a provided message. That's it. If you're merging a branch, within your code you can do a log check and pump each message in and load up all the invalid ones to throw an error about if needed or pass if successful on your own.

I feel like we should aim to minimize the git knowledge contained within the repository here. It should focus as clearly as we can on validating a message regardless of where it is from. And using a git commit hook is simply a nice extension to do it automatically with.

[cmalard]

In fact, it is quite complicated to check all new commits on a branch. I've managed to do it in bash, take a look:

SEPARATOR='|'
MSGS=$(git log develop..HEAD --pretty="format:%B$SEPARATOR" | sed -e ':a' -e 'N' -e '$!ba' -e "s/\n$SEPARATOR\n/$SEPARATOR/g" -e "s/\n$SEPARATOR//")
IFS=$"$SEPARATOR";
for msg in $MSGS; do
  npx validate-commit-msg "$msg"
done
unset IFS

This is not simple, even if we provide this script in the README to help users. And this is tricky, one character | is used to split the commits. Not bullet-proof. A command like the one proposed validate-commit-msg --from=develop would be a great help.

This is the same approach with https://github.com/conventional-changelog/validate-commit-msg/issues/101#issuecomment-318801035: once the error commit is saved, we should provide a tool to restore it at the next commit, because the saved file would be specific to validate-commit-msg. That would be hard for everyone to do it individually.

To adress your concern about the increasing git knowledge in the repo, we could have in target to separate the git-specific code as much as we can, WDYT ? :slightly_smiling_face:

[stevemao]

I feel like we could make a seperate module which does it.

[cmalard]

How would you do it ? Use it ? Not sure to see what you guys think about.

[stevemao]

Yeah I'd create a wrapper around this module. I mean I'd keep this module small and focused. Any extra features like this could be in another module which just simply wraps this one.

[cmalard]

As I see it, this module will be refactored to use commitlint or conventional-commits-parser. So all the validation process would go out of the scope of validate-commit-msg, it would become a cli tool only.

In this scenario, I would expect it to fill these tasks:

• validate each commit message with pre-commit hook
• provide previously discarded message at the next attempt
• validate all new commits in my CI

All these with simple to use commands. And only one module to install and keep updated.

Don't you think ?

[stevemao]

And only one module to install and keep updated.

I definitely think its better. The wrapper could just expose all methods this module has. The important thing is how the architecture looks like after the refactor. Does it make sense to add such feature in the codebase, etc.

[Garbee]

I feel kinda weird having this functionality in the package. The validator handles one task and does it well enough. The problem is this is asking for the validator to take responsibility of this need now and to maintain it in the future. Which includes making sure it works consistently across platforms and different environments. That's not as easy as it may seem at first.

I have no problem documenting the bash example, as an example, for validating all commits in a CI system for a merge. It's extremely useful to provide that. Including the code within our package and making a CLI option so it's baked in for users, I'm not keen on. This feels very icky of scope creep to me which I'm a big fan of avoiding.

So, I'm going to close this out since it isn't something I feel comfortable accepting into the core package at the moment.

The path forward to get this kind of feature included is to make your own package that makes this use-case easy for developers. This is making a wrapper as @stevemao is mentioning. So they'd call your CLI endpoint instead of this in their merge request CIs. Then, once it's seen some battle from external users and it's gained some traction. You can come back and provide a more detailed explanation on how useful it is and we can see it's seen actual use and see a history on maintenance/support to analyze if it is worth bringing in-package.

Also, the refactor isn't "making this a CLI tool only". I'm looking at simply pushing the logic of abstracting the commit message into an object to get interpreted out. There's still all the configuration and analyzing of that commit object that gets done internally. That is why this package exists, to validate a message based on the configuration provided. It doesn't necessarily need to also handle the workload of parsing the message out. That was needed when it was created, but not anymore.

[cmalard]

Well, if I make my own package / fork this project, I do not see the benefit for the community. The whole point of #84 was to merge efforts & tools. commitlint could handle the validation. validate-commit-msg is already a CLI tool. Let's continue toward this path together, not divided.

[Garbee]

Let's continue toward this path together, not divided.

Looking over commitlint, what is this project providing that it doesn't already do?

Looks to me like the path forward together is best to deprecate this project and forward people to commitlint.

We never asked you to fork the package, we asked you to write a wrapper tool that depends upon this one for validation in which you then provide the functionality you think others will want. To battle test it in various scenarios without affecting this packages releases. Then once we've seen its fairly stable and usable in various forms. It could be assessed being pulled into core in some fashion for the whole audience.

92 is EXACTLY why I want us to reduce the amount of git knowledge internally. Any mods to try and fix bugs for some end up causing others. This stuff isn't easy to write unit tests for to verify it works. Therefore I'd like to leave what can't be easily tested up to the application authors to handle in their own way for their project's needs.

[cmalard]

Just saw this nice GitHub App : https://github.com/tlvince/validate-commit-msg-bot

It is not configurable but does the job if you stick to the default config.