Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide backwards config compatibility #133

Closed
ghost opened this issue May 29, 2017 · 4 comments
Closed

Provide backwards config compatibility #133

ghost opened this issue May 29, 2017 · 4 comments

Comments

@ghost
Copy link

ghost commented May 29, 2017

When ghooks deprecated in favor of this project it left a lions share of manual work. While it may seem inconsequential to the owners and collaborators of this project, it would be less work for husky to provide backwards compatibility than the man-hours which will be spent updating configuration.

For context, here are the current download statistics for the popular ghooks library:

screen shot 2017-06-04 at 6 38 19 pm

The above metrics signify just some of the many individuals which'll need to update their package manifests as the NPM deprecation notice drives upgrades to husky.

Backwards compatibility has the added benefit of allowing users to keep their scripts free and clear of cruft dev dependencies might want to add because NPM does not support the likes of an extra field in the package manifest.

As an alternative, and something standard does, you may want to consider providing a husky object at the root of the package manifest to help eliminate the need for clobbering scripts until NPM provides an appropriate solution for the problem current design attempts to achieve by eschewing use of package.config.

And as an aside, it is my opinion reserving scripts as a sacred ground specifically for package author use is important, and that's something this library currently does not provide.

It would be a pleasure to hear the community weigh in.

@ghost
Copy link
Author

ghost commented Jun 18, 2017

No traction. Closing.

@ghost ghost closed this as completed Jun 18, 2017
@ghost
Copy link
Author

ghost commented Jun 19, 2017

@typicode fyi - ghooks-org/ghooks#209

@typicode
Copy link
Owner

Hi @JHabdas,

First, sorry for the lack of answer. To be honest, I don't always have the time to reply to all (even though I would really like to!) but I try to address them at some point or the community does.

For ghooks compatibility, actually I didn't see the benefit of supporting ghooks fields in husky, since they are 2 different projects and would force to maintain 2 styles of configuration.
Also I may miss something, but switching is just a matter of updating 2 or 3 lines?

husky could automatically update package.json to move ghooks scripts to scripts, but I would feel it kind of intrusive for a package to do that (maybe it's not but I'm not aware of tools that do that automatically).

That said, to make it easier for users, existing ghooks scripts in .git/hooks are automatically migrated.

Regarding where to put them, I prefer them to be with other npm scripts and npm hooks (but that's a personal opinion).

If ghooks suits you better, that's cool :) 👍

Regarding another topic that was raised, precommit vs pre-commit syntax, I'm considering switching to the latter but I'm currently working on a new version in next branch which should improve some other things

@ghost
Copy link
Author

ghost commented Jun 19, 2017

For ghooks compatibility, actually I didn't see the benefit of supporting ghooks fields in husky

Sorry if it wasn't clear, but my main concerns were actually easing the upgrade path and not clobbering scripts. That could also be achieved by simply changing pkg.config.ghooks to pkg.husky and respecting the current ghooks format (could be automated, but trivial if manual).

I would feel it kind of intrusive for a package to do that

Agreed.

That said, to make it easier for users, existing ghooks scripts in .git/hooks are automatically migrated.

Nice touch.

To be honest, I don't always have the time to reply to all...

Hope you're able to prioritize the migration over feature development assuming that's where most users could use you at present.

If ghooks suits you better, that's cool

I'd rather be using the more feature-rich and well-supported product, personally. And that's why I tried husky but as mentioned above I don't feel right updating the scripts object. Updating the scripts object would result in stuff like this:

"scripts": {
  "pre-push": "npm run lint",
  "lint": "standard"
}

Which seems to have a design smell which could lead to:

  • Potential build breakage while taking a major (reasonable, but perhaps not expected).
  • Individuals spending time trying to figure out the origin of the magic hook scripts.
  • Possible removal by individuals not understanding why those scripts are there.

Perhaps a husky:pre-push script, etc., would mitigate the concerns above, if not creating a pkg.husky. But only time will tell I suppose.


At any rate, best of luck with the upgrade. Looking forward to learning from husky over time.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant