Skip to content
This repository was archived by the owner on Feb 5, 2018. It is now read-only.

Investigate commitlint differences #110

Closed
Garbee opened this issue Aug 3, 2017 · 32 comments
Closed

Investigate commitlint differences #110

Garbee opened this issue Aug 3, 2017 · 32 comments
Assignees

Comments

@Garbee
Copy link
Collaborator

Garbee commented Aug 3, 2017

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.

@Garbee Garbee self-assigned this Aug 3, 2017
@cmalard
Copy link
Contributor

cmalard commented Aug 3, 2017

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
Copy link
Collaborator Author

Garbee commented Aug 3, 2017

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

@stevemao
Copy link
Contributor

stevemao commented Aug 4, 2017

CC @marionebl

@marionebl
Copy link

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
Copy link
Collaborator Author

Garbee commented Aug 4, 2017

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
Copy link
Contributor

stevemao commented Aug 8, 2017

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
Copy link
Collaborator Author

Garbee commented Aug 9, 2017

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
Copy link

travi commented Aug 12, 2017

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
Copy link
Collaborator Author

Garbee commented Aug 12, 2017

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

@travi
Copy link

travi commented Aug 14, 2017

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
Copy link
Collaborator Author

Garbee commented Aug 14, 2017

@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
Copy link

travi commented Aug 14, 2017

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
Copy link
Collaborator Author

Garbee commented Aug 14, 2017

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
Copy link

travi commented Aug 14, 2017

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
Copy link

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
Copy link

iamstarkov commented Aug 15, 2017

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 [email protected]: 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
Copy link

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

@iamstarkov
Copy link

it took me half an hour to find out relevant replacement

@iamstarkov
Copy link

is there any migration guide?

@danielo515
Copy link

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
Copy link
Collaborator Author

Garbee commented Sep 26, 2017

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
Copy link

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
Copy link

@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
Copy link

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
Copy link
Collaborator Author

Garbee commented Sep 27, 2017

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
Copy link

@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
Copy link

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
Copy link

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
Copy link
Collaborator Author

Garbee commented Sep 27, 2017

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
Copy link

@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
Copy link
Collaborator Author

Garbee commented Nov 19, 2017

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
Copy link

hutson commented Feb 5, 2018

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

@hutson hutson closed this as completed Feb 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants