observing / pre-commit

Automatically installs a git pre-commit script in your git repository which runs your `npm test` on pre-commit
MIT License
1.88k stars 150 forks source link

Support other git hooks #25

Closed danvk closed 7 years ago

danvk commented 9 years ago

e.g. pre-push. It seems silly to have separate projects (e.g. https://www.npmjs.com/package/prepush-hook) for each different hook.

longlho commented 9 years ago

:+1: for this. Happy to submit a PR of the owner is cool w/ it @3rd-Eden

3rd-Eden commented 9 years ago

Well, it would be kinda weird to have as the suggested functionality doesn't match the name of the module. But I having tons of separate modules is silly as well. I guess we could install the various of hooks based on the contents of the package.json. Thoughts on this @Swaagie ?

Swaagie commented 9 years ago

Could also push the same project to two different names, e.g. one to pre-commit and one to pre-push and have a prepublish script here that sed an env variable in https://github.com/observing/pre-commit/blob/master/hook, or is that wack ;)

danvk commented 9 years ago

That's wack :)

I wouldn't find it jarring for a project named pre-commit to support other git hooks. In fact, I was surprised that it didn't (hence this issue).

If the name is bothersome, though, I'd suggest renaming this package to git-hooks or some such, and deprecating the old name.

brycefisher commented 9 years ago

Ditto on that last suggestion. I was just about to open this issue myself.

I prefer the git-hooks name over pre-commit if the plan is to support multiple git-hooks. Otherwise, it's :fork_and_knife: time ;-)

danvk commented 9 years ago

Unfortunately another npm package already took the name git-hooks

robhrt7 commented 9 years ago

So have you decided on something, which module is recommended for setting up pre-push hooks?

jpodwys commented 9 years ago

Also curious whether a conclusion has been reached.

3rd-Eden commented 9 years ago

@jpodwys I'm fine with supporting addition hooks, but with some addition constraints that do not apply to the "core" hook of this project, the pre-commit hook.

This eliminates any accidental hook overrides for existing users and prevents multiple npm test executions. As for the progress on this work, i'm accepting contributions here as my own personal schedule is quite full (read overflowing with work ;p) atm so it would take some while before I can find some time to add this all in.

fatso83 commented 8 years ago

Why try to cram all of this into this one module anyway? I'd rather see a kind of githooks-core package that supported all the common plumbing, and then let that be used by various modules for custom behaviour. Having all the historic baggage of this package affect other hooks seems silly. This package could then depend on core.

edit: all of this is solved by husky

mpareja commented 8 years ago

:+1: to what @fatso83 said.

sandy-adi commented 8 years ago

The pre-push hook is an exact copy of this repo except for a find and replace of commit with push. It might not be necessary to have different modules for this as of now. Cause essentially you would just be adding a config to enable push hooks.

bumbu commented 7 years ago

typicode/husky seems to do that (supports most/all git hooks)

vadimcoder commented 7 years ago

Oh thank you @bumbu very very much for the link. It solves the problem.

quantuminformation commented 6 years ago

any update on pre-push added to this?

fatso83 commented 6 years ago

@QuantumInformation Just use a package dedicated to the problem, such as husky mentioned above.

quantuminformation commented 6 years ago

already switched to husky thx