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
557 stars 100 forks source link

Investigate commitlint differences #110

Closed Garbee closed 6 years ago

Garbee commented 7 years ago

I'd like to have a detailed look at commitlint and see what their offering is compared to this project. It seems to me like for the main points they have a much better CLI interface for users. Depending on where the offerings differ we may want to wrap commitlint or if they are similar enough approach with suggestions to include our offerings into their system. If they agree and will take PRs, we could get that into a state where this project could become deprecated and recommend commitlint instead.

I think our offerings are very similar. I'd rather we not duplicate too much effort in the community if there is a better offering that equals the feature set.

cmalard commented 7 years ago

If it appears that comitlint could handle all the lint, we should consider wraping it and convert validate-commit-msg to a full-feature cli that could handle Git and all that things far away from a linter :-)

Garbee commented 7 years ago

Specifying git specific stuff more heavily IMO would just be a whole new project.

stevemao commented 7 years ago

CC @marionebl

marionebl commented 7 years ago

Hey there, I'm the author of commitlint. Currently I'm the only maintainer of the associated @commitlint packages. I'd be glad to join efforts on this and grant interested contributors maintainer status on the repo.

As far as I can see commitlint and validate-commit-msg are very similiar. What I noticed immediately are the different configuration concepts:

commitlint borrows the shareable configuration concept from eslint and uses commitlint.confg.js files to enable computed and async configuration. validate-commit-msg uses *rc and package.json files (think cosmiconfig?)

Garbee commented 7 years ago

Yea, the config changes aren't that difficult to work though. It seems to me looking it over that all the base functionality is there.

Overall I personally think marking this package as deprecated/abandoned whatever the NPM term for it is and pointing to commitlint is the best path forward. So the community can focus on fewer tools to handle this simple task and we can improve the UX which you already have done to a great degree.

cc @stevemao Any thoughts on this since you seem to be the head in the conventional-changelog org? Is there any particular reason to try and keep validate-commit-msg alive and in active development when commitlint can handle the same functionality and provides a better UX right now? I think as far as any functionality differences that may be found we can work towards getting what is necessary in commitlint.

stevemao commented 7 years ago

Is there any particular reason to try and keep validate-commit-msg alive and in active development when commitlint can handle the same functionality and provides a better UX right now?

No. I simply didn't know commitlint. Let's merge them then :)

Garbee commented 7 years ago

SGTM. I'll research the path tomorrow afternoon. Should be fairly straightforward. Migration guide, mark as deprecated, point to commitlint. But, still seeing how other projects handled these kinds of things for any other cautions to watch for.

travi commented 7 years ago

I'd recommend adding some sort of messaging to the npm and github readmes if the plan truly is to deprecate. It's a bit confusing to see the deprecation message from npm and find no evidence that it is legit from the project until finding this thread.

Garbee commented 7 years ago

Yea, I'm working on it. Just got the deprecation message up on NPM first. Then got distracted with other work.

travi commented 7 years ago

one difference that i've noticed after switching a few projects is that ability to use WIP to skip validation does not seem to be available. is there a plan to port missing features like this?

Garbee commented 7 years ago

@travi Please bring up any issues with Commitlint.

IMO that type of thing belongs in your scripting to determine whether or not to validate though. But, that's just my opinion on how this kind of thing should work.

travi commented 7 years ago

sure thing. i opened https://github.com/marionebl/commitlint/issues/60, but thought it should also be included here too, since your team was evaluating differences.

it seems like an insignificant feature, but my team has found it to be very valuable and i think it would be somewhat complex to implement separately in every one of our repos. worst case, i could create a separate cli tool to handle this, but it seems better to keep it as part of this project, even if some choose not to leverage it.

Garbee commented 7 years ago

i think it would be somewhat complex to implement separately in every one of our repos.

This is where you'd create a package to require for your team's/company's CI system and require it to easily share and re-use the scripts. But, these packages should stick to validating the messages alone and any extra work be put into users hands. That keeps the packages minimal and easy to maintain and provides developers full control over what goes in and how to handle failures.

travi commented 7 years ago

This is where you'd create a package to require for your team's/company's CI system and require it to easily share and re-use the scripts.

Understood...

worst case, i could create a separate cli tool to handle this, but it seems better to keep it as part of this project, even if some choose not to leverage it.

i'm all for keeping tools minimal, but i'm suggesting that this seems to me to be a bit more core than simply "extra work". there are already some skip rules built into commitlint.

plus, if you went as far as configuring ci to reverify there would be even more "extra work" (if even possible since it expects a commit range) left up to the consumer if the tool doesn't know about the commits that should be ignored.

i appreciate making sure additions are justified, but i personally do think this one still makes sense to include.

anyway, to keep this from further diverging from the conversation in the commitlint issue, its probably best to include further thoughts in marionebl/commitlint#60

marionebl commented 7 years ago

plus, if you went as far as configuring ci to reverify there would be even more "extra work" (if even possible since it expects a commit range) left up to the consumer if the tool doesn't know about the commits that should be ignored.

Let's automate this in a small lib/cli combo

iamstarkov commented 7 years ago

Yea, I'm working on it. Just got the deprecation message up on NPM first. Then got distracted with other work.

It is confusing as hell.

➜  / npm repo validate-commit-msg       
npm WARN deprecated validate-commit-msg@2.14.0: Check out CommitLint which provides the same functionality with a more user-focused experience.
➜  / npm repo CommitLint
npm ERR! code E404
npm ERR! 404 Not Found: CommitLint@latest

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/vlasta/.npm/_logs/2017-08-15T13_49_00_521Z-debug.log
➜  / npm repo commitlint
npm ERR! code E404
npm ERR! 404 Not Found: commitlint@latest

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/vlasta/.npm/_logs/2017-08-15T13_49_06_249Z-debug.log
iamstarkov commented 7 years ago

pls make change deprecation message to contain a link to a repo

iamstarkov commented 7 years ago

it took me half an hour to find out relevant replacement

iamstarkov commented 7 years ago

is there any migration guide?

danielo515 commented 7 years ago

I am also interested on a migration guide. I use this package to validate commit messages using husky and I can't see how commitilint can be a replacement

Garbee commented 7 years ago

Commitlint at the core does the same validation system this package does. You simply have to do some extra scripting around it for CI and git hooks. I'm trying to make the time to write a migration guide to help out with getting information on how around. Removing the built in scripting saves a lot of extra code to maintain in the package that doesn't strictly need to be there. Which has lead to buggy behavior for some.

danielo515 commented 7 years ago

Then it's not the same solution. I just want to add a dependency and put it inside my commitmsg hook, nothing else. Saying that this package is deprecated and pointing people to another package that has "something" in common with this one but that is not a direct replacement is confusing. Personally I will not use any of those, I'll leave both in favor of a more simple and not deprecated solution. I have dozens of projects to maintain and it's impractical to build even small scripting for each one . Regards

marionebl commented 7 years ago

@danielo515 Could you point at what exactly is missing for your use case and what that use case is (lint on commit, lint on CI, lint PRs)? I'm the author of commitlint and never used validate-commit-msg, so the differences are rather fuzzy for me. I am pretty sure we can easily make the transition smoother with simple stuff, so your input is greatly appreciated.

danielo515 commented 7 years ago

Hello @marionebl I'm just interested in commit message validation using angular style. The way I was doing this with validate-commit-msg was through husky. It was as simple as this:

"scripts": {
"commitmsg": "validate-commit-msg"
}

Is that possible with your tool ?

Garbee commented 7 years ago

I have dozens of projects to maintain and it's impractical to build even small scripting for each one .

If you're re-using the exact same scripting, then make a package to share it between projects.

The goal of the package is to validate a message structure. Not to handle the individual niches of each persons desired workflow with git (or any other version control system.) The git logic is becoming increasingly complex as people want the package to handle more robust cases. All of which are valid but also lead to more code that can be buggy in different desired contexts.

The best way to prevent that was going to be in V2 to remove the git logic. Leaving that up to developers to handle. Then it will also work right away with any other source control system people may use. In some of the discussion on the v2 thoughts, I came across commitlint and saw it had a better UX and the same basic functionality I was wanting for V2. So, I decided instead of having a rift in the community, simply support another superior user-facing package. That way everyone can focus on making one project the best it can be instead of having multiple.

Nothing is stopping the existing package from working as it is. If this works for you then things should continue to work well into the future. No reason to change things. However if you're hitting one of the more complex use cases people wanted to be solved internally, such as validating all commits in a merge request, then you'd need to do custom scripting anyways. So, moving to something like commitlint is the best choice there. Writing a solid wrapper of scripts that can be shared between projects.

marionebl commented 7 years ago

@danielo515, this should do the trick for local linting with husky:

npm install @commtlint/cli commitlint-config-angular
{
  "scripts": {
    "commitmsg": "commitlint -x angular -e"
   }
}
marionebl commented 7 years ago

I'm with @Garbee on separation of concerns here. As I personally use commitlint a lot on Travis and Gitlab CI I might come around wrapping up the more elaborate scripts as plug and play npm package + cli

danielo515 commented 7 years ago

The goal of the package is to validate a message structure.

And that is the usage I was giving to it. I didn't need anything simpler nor more complex

Nothing is stopping the existing package from working as it is. If this works for you then things should continue to work well into the future

Yes, a big deprecation warning on the very top of your repository.

Garbee commented 7 years ago

And that is the usage I was giving to it. I didn't need anything simpler nor more complex

Not exactly. You were also desiring us to handle all the git logic on your behalf within the same project. Which is messy and leads to difficult to track problems and refactoring.

Yes, a big deprecation warning on the very top of your repository.

I'm not sure what you mean here. If you're asking we add a warning, yes I need to find the time to do that. If this is complaining about the warning in the CLI when you install the package, well it is a warning. It is not an error nor does it stop things from functioning. It is simply an indicator that it is no longer being maintained.

marionebl commented 6 years ago

@Garbee I finally carved out some time to write a migration guide for validate-commit-msg users.

It is available here: http://marionebl.github.io/commitlint/#/guides-upgrade?id=validate-commit-msg

Could you link there from the deprecation warnings? Thank you!

Garbee commented 6 years ago

That's awesome. Thanks @marionebl. Yea I'll get it linked in the readme here later today while I'm also updating the npm message.

hutson commented 6 years ago

This package has been deprecated. Please use https://github.com/marionebl/commitlint instead.