search

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

sourcing .bash_profile clash with nvm #33

Closed mcollina closed 9 years ago

mcollina commented 9 years ago

If you are developing using nvm, sourcing the bash profile will change which node version are you targeting. This means that if you are developing a native addon against iojs and your default is 0.10, everything falls apart.

A quick solution for this might be checking if node is already present, and if so do not source. I will send a PR with this fix.

Side note, can you please bump the MINOR or at least the MAJOR when you change the hook? It broke a lot of stuff for the way I work.

3rd-Eden commented 9 years ago

@mcollina I never expected that such small change would break things. So I did patch bump as it actually fixed broken installations of pre-commit. So semver wise, it made sense to do a patch release. And yes, checking for Node before sourcing might be a better approach here.

mcollina commented 9 years ago

Yes, I agree! It's more a suggestion: given how critical that script is, it's probably worth a minor bump every time is touched, just in case.

If some people have an issue with the current version, they can always upgrade. In this way, nobody gets hurt. Il giorno mer 10 giu 2015 alle 14:44 Arnout Kazemier < notifications@github.com> ha scritto:

Closed #33 https://github.com/observing/pre-commit/issues/33 via #34 https://github.com/observing/pre-commit/pull/34.

— Reply to this email directly or view it on GitHub https://github.com/observing/pre-commit/issues/33#event-327383359.