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

Consider deprecating in favor of husky? #166

Closed
kentcdodds opened this issue Sep 2, 2016 · 41 comments
Closed

Consider deprecating in favor of husky? #166

kentcdodds opened this issue Sep 2, 2016 · 41 comments

Comments

@kentcdodds
Copy link
Contributor

Hey friends! So I recently found another similar module called husky. I've tried it on a few of my projects and really like it. Here are a few reasons I think it'd be great to deprecate ghooks and put our efforts behind husky:

  1. It's good to merge projects/choose a "winner" when that can reasonably be done and focus efforts
  2. They support git GUIs Add support for GUIs #40
  3. Config is slightly simpler

Thoughts? 💭

@kentcdodds
Copy link
Contributor Author

I'm personally going to stop working on ghooks in favor of husky. Removing myself from the list of collaborators and unwatching. Let's merge efforts.

@okonet
Copy link

okonet commented Dec 1, 2016

Not sure it's possible to do something like this with husky though:

https://gist.github.com/okonet/9706ec3b7f29e05109f3744fbc1fc91e

@kentcdodds
Copy link
Contributor Author

Thanks for bringing this up. I had to do a little research, but I found that husky will store the parameters in an environment variable called GIT_PARAMS which you can access in your script 👍

Honestly, it's not quite as nice IMO, but it's not a terrible solution, and there aren't a lot of use cases you need those args so I'm good with it.

@gtramontina
Copy link
Collaborator

gtramontina commented Dec 2, 2016

Sorry, I've never got back to this one… <insert lame excuse here 😊>

Yeah, I'm definitely in favour of sticking with a "winner". Actually, I'm looking for someone to maintain this project. By maintain I mean to support those who have to stay on ghooks (for whatever reason) until they move out. I'm going to spare some time to tidy this repo up and update the readme to mention that…

As far as passing the parameters to the script, perhaps this is the opportunity to collaborate and makeapullrequest.com to husky. 😄

@kentcdodds
Copy link
Contributor Author

I'm going to spare some time to tidy this repo up and update the readme to mention that…

Sounds great!

As for looking for another maintainer, I don't know what more help people need. Luckily a migration from ghooks to husky is pretty simple and straightforward (with the exception of the parameters thing, which most people aren't using).

@thecotne
Copy link
Contributor

@kentcdodds @gtramontina please add deprecation warning in readme so newcomers know that husky is more supported and maintained solution

@kentcdodds
Copy link
Contributor Author

Good idea @thecotne. Fancy making a PR?

@AdriVanHoudt
Copy link

There should be a notice on how to move to husky, as in how to uninstall the ghooks git hooks before installing husky

@kentcdodds
Copy link
Contributor Author

True, would you like to make a pull request?

@AdriVanHoudt
Copy link

I don't know what the right solution is. Deleting all your git hooks seems dangerous (unless you know what you're doing), asking the rest of my team to do that is also tedious.

@kentcdodds
Copy link
Contributor Author

Anyone using ghooks or husky will be managing all of their git hooks via the tool and will most likely not have any custom git hooks. So we could probably simply say:

To migrate from ghooks to husky, delete all of your git hooks first by deleting the .git/hooks directory (if you have any custom hooks, you should preserve those by deleting all other hooks individually). Then uninstall ghooks and install husky.

We could take it a step further and add a postuninstall hook to ghooks that deletes all of the hooks that are generated by ghooks. That'd make the process and instructions easier. But I'm afraid I can't commit any more time to this package.

@kentcdodds
Copy link
Contributor Author

Also, I'm going to go ahead and close this because the deprecation warning is good enough for me.

@AdriVanHoudt
Copy link

@kentcdodds would you be ok with a PR to add that text to the readme or is that overkill on the deprecation notice?

@kentcdodds
Copy link
Contributor Author

I'd be fine with that, but I'm no longer able to merge pull requests.

@AdriVanHoudt
Copy link

@gtramontina do you have an opinion on this?

st3v3nhunt added a commit to st3v3nhunt/service-etl-function-app that referenced this issue Dec 30, 2016
st3v3nhunt added a commit to nhsuk/connecting-to-services that referenced this issue Dec 30, 2016
st3v3nhunt added a commit to nhsuk/connecting-to-services that referenced this issue Dec 30, 2016
@gtramontina
Copy link
Collaborator

Hey hey… sorry about the silence. Yeah, absolutely! I'm also noticing a few references to this issue. I'll get @kentcdodds' message, which looks just fine, and reference one of @st3v3nhun's commits as an example. Thank you all!

@jamietre
Copy link

jamietre commented Jan 12, 2017

Just wanted to make an observation to those considering switching (which is what led me here) - husky does not seem to play well with Windows since it works by generating shell scripts; indeed there's an open issue.

Anything that depends on a shell environment is going to be tricky to make work reliably on windows. There's no guarantee a windows user even has git bash installed at all.

@typicode
Copy link
Contributor

Well.. I didn't mean to just drop the mic after that post...

No worries :) just wanted to take time to better understand this case. So thanks for the details.

Actually, even in CMD, shell scripts started with Git will work:

C:\Users\typicode\git-test>ls # will of course fail
# .git/hooks/pre-commit
# #!/bin/sh
# ls
C:\Users\typicode\git-test>git commit -am test # will correctly output ls

And so this works in IDE too, as Git will run hook scripts and they won't be run directly in CMD. I've tested it with Visual Studio Code which comes with Git built-in.

Also AFAICT from Windows related issues, hook scripts are always run. When they failed on Windows it was because of small differences like path that needed to be normalized.

I would suggest trying husky with some of your work configuration. It's easy to remove if it doesn't work for you or some of your team.

npm install husky # add --save-dev if you want to make it available to your team
npm remove husky # will remove hooks that husky installed

Existing hooks won't be touched.

mac GUI issue and nvm

This affects people that have installed Node using only nvm on OS X or Linux.

With nvm, PATH is only modified in the terminal, so when you start a GUI app it will get a different PATH and won't be able to find Node.

In this configuration, if hook scripts are written using Node it won't work. With a shell script it's easy to load nvm and node.

Also I think running a shell script is slightly faster than loading Node.

node instead of shell script

On OS X and Linux, I prefer to keep shell for the previous reason.

On Windows not against it and it may be a good idea, but I would like to have at least a failing scenario on this platform (install tool X, run command Y, ...) before creating a specific hook script.

Hope it clarifies things and that I didn't miss the point.

tarmolov added a commit to tarmolov/git-hooks-js that referenced this issue Jan 22, 2017
@jamietre
Copy link

jamietre commented Jan 23, 2017

This is interesting. So it seems possible my central concern may be, er, "alternate facts" because what I didn't consider is that the scripts are always launched by git, rather than from a node process which will spawn a new default command processor via the OS. I might still argue that parsing package.json is trivial enough to do with node that it still is sensible, generally speaking, but if we're already in a bash script then maybe it's pointless.

I still have a bit of a nagging feeling but maybe it's not a big deal (at least not for us). There are certainly scenarios that could potentially be incompatible. For example, if you are using a git client other than Git for Windows, e.g. git package for msys2. But frankly this is edge-casey enough that it's probably not worth a major philosophy change to support.

I'll give husky a try in a few different configs to be sure I'm not overlooking anything else here. But assuming I can't blow it up at a fundamental level, if the open windows issue is simply a path parsing problem then consider my concerns allayed.

@typicode
Copy link
Contributor

git package for msys2

If it can help, it seems it works too typicode/husky#70 but I've not tried it myself.

@jamietre
Copy link

jamietre commented Jan 26, 2017

The scenario I can imagine is that you're running git in an environment where it can't find the shell. I don't know how git integrates with the shell when it invokes it on commits (but obviously it's not through windows -- or it wouldn't work anywhere), but I was able to make it fail when using msys2 with a cmd shell like this:

install MSYS2
install git: pacman -S git
launch a CMD shell
remove git for windows from your path (if it's also installed)

Commit hooks are ignored. Strangely it seems to work fine with TCC/LE? I don't really know why this would behave differently in TCC vs CMD

I am really not worried about this. I actually am almost that edge case... I use MSYS2 for bash commands on windows, and TCC/LE for my shell (which is not bash but CMD replacement). I do this because I like the MSYS2 package manager and the availability of all the tools that don't come with Git for Windows, and TCC/LE is far better than the git & msys2 bash shells for usability. But at the end of the day I have a hard time imagining someone being enough of a hacker that they want to use MSYS2, and using MSYS2 git, and using it on CMD instead of a shell replacement.

I am satisfied that the fundamental problem I feared (scripts not working because default windows command shell isn't bash) doesn't exist... and I suspect that anyone who was trying to work in the contrived scenario I created would have other problems anyway.

@uglow
Copy link

uglow commented Jan 31, 2017

I've tried upgrading a ghook commit-msg hook to husky, and I'm finding that the commit message isn't passed to the Node script as it was when using ghooks.

This works using ghooks:

"config": {
    "commitizen": {
      "path": "node_modules/cz-customizable"
    },
    "cz-customizable": {
      "config": "config/release/commitMessageConfig.js"
    },
    "ghooks": {
      "commit-msg": "node ./node_modules/cz-customizable-ghooks/lib/index.js $2",
    }
  },

Note the $2 parameter which is the path to the file containing the commit message.

When using husky, no additional arguments seem to be being passed to the script, so it fails. So at the moment, husky is NOT a like-for-like replacement (or enhanced version) of ghooks.

CC @typicode.

@typicode
Copy link
Contributor

hi @uglow,

You should be able to find additional arguments in GIT_PARAMS environment variable.

@duclet
Copy link

duclet commented Feb 23, 2017

Hmm, just stumbled upon this news today. I have used husky in the past and went with ghooks because I felt it was better designed. The biggest flaw for me with husky is that it put the scripts inside of the scripts block. That makes it conflicts with how NPM automatically runs pre and post versions of that command as well. That being such an integral part of how husky works though, I doubt it can be fixed. I wouldn't mind taking over this project.

@gtramontina

@gtramontina
Copy link
Collaborator

@duclet I'd be more than happy to move this repo into an organization and add you as contributor. Let me know!

@kentcdodds
Copy link
Contributor Author

I imagine that @typicode may entertain the idea of allowing config in a similar way that ghooks had if it means there's more effort behind husky rather than split-effort between two tools. Has anyone asked?

@duclet
Copy link

duclet commented Feb 28, 2017

@gtramontina, that would be great.

@kentcdodds, there are two open tickets relating to this matter that doesn't seem like it is going anywhere anytime soon.

@jamietre
Copy link

jamietre commented Feb 28, 2017

I think there is room in the world for more than one project with similar goals. Competition is the catalyst of innovation right?

@duclet In addition to your design concerns, even if possible to do with the shell, I still think that it's just common sense to use Node to do file system stuff rather than bash commands. I'm glad you are keeping this alive. Officially canceling plans to migrate.

@typicode
Copy link
Contributor

typicode commented Mar 1, 2017

Hi,

Just replied here typicode/husky#99
feedback about usage and conflicts is welcome

@duclet
Copy link

duclet commented Mar 8, 2017

I still have concerns but I did make a recommendation in that ticket which I can live with. We'll see how it goes but if not, I would still like to take over this project if possible. Thanks.

@ghost
Copy link

ghost commented Jun 18, 2017

@duclet I waited 20 days for a husky maintainer to acknowledge my concerns and nada. I've decided to return to using ghooks library with justification: typicode/husky#133

@gtramontina Please offer to transfer ownership of this repo and the module. I really don't need a script called precommit/prepush to run my script called lint. Don't make me do stuff.

@ghost
Copy link

ghost commented Jun 19, 2017

@duclet please lmk if you'd like an invite to the ghooks-org @gtramontina set up so we can revive this puppy.

@ghost ghost mentioned this issue Jun 19, 2017
@gtramontina
Copy link
Collaborator

gtramontina commented Jun 19, 2017

Alright…

I've just moved ghooks to its own org ghooks-orgghooks was already taken – and invited those who expressed interest in keeping ghooks alive to be part of it. cc/ @jhabdas @duclet @ta2edchimp

I still have to figure out npm ownership transfer. Tips and hints are welcome! npm owner add <user> <pkg> does the trick, so I've added @jhabdas as owner for now.

Hope this helps the community! 🍻

(had this message sitting here when my battery died)

@gtramontina
Copy link
Collaborator

By the way, I got the logo from https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team -- we should probably add an attribution somewhere (or come up with a new one – anyone with design skills? 😄)

@ghost
Copy link

ghost commented Jun 19, 2017

@gtramontina It's not super usual for git projects to do this yet, but it might be fun to set up a donation request form to a bitcoin account and hope to score some coin for your efforts.

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

No branches or pull requests

9 participants