Open jsumners opened 7 years ago
@jsumners I can't reproduce the issue. Can you try a few things:
Delete node_modules/
and run npm install
, just to make sure it's not a silly npm issue. (Don't use yarn or alternate package managers, please)
What versions of node and npm? Please share the output of node -v
and npm -v
.
Actually, I can reproduce the issue when I make a commit, so it points to this being a pre-commit
issue.
Running snazzy
directly via npm run lint
or ./node_modules/.bin/snazzy
produces the correct exit code of 1
.
Glad you were able to reproduce it @feross . I was going to say that I created the test case from nothing and I don't use anything other than npm.
@jsumners I suppose one other useful question is: did this ever used to work in the past? That is, did a recent version of pre-commit
or snazzy
cause this issue to start happening?
@feross I do not know the answer to that question.
@jsumners I see, so you just tried it for the first time and it didn't work for you.
Could this have something to do with the way that we set the exit code in the process.on('exit')
event?
https://github.com/feross/snazzy/blob/f2172ffeeecc3d0d8efdad8b7149fa953c8a2327/bin/cmd.js#L20-L24
@jsumners In the meantime, you can try piping from standard
to snazzy
instead, like this:
standard --verbose | snazzy
@feross piping with --verbose
produced the desired result:
% git commit -m 'test 2' [s:0 l:8172]
standard: Use JavaScript Standard Style (https://standardjs.com)
standard: Run `standard --fix` to automatically fix some problems.
/private/tmp/06/precommit-test/index.js
3:10 error 'invalid' is defined but never used no-unused-vars
3:17 error Missing space before function parentheses space-before-function-paren
5:2 error Newline required at end of file but not found eol-last
✖ 3 problems
pre-commit:
pre-commit: We've failed to pass the specified git pre-commit hooks as the `lint`
pre-commit: hook returned an exit code (1). If you're feeling adventurous you can
pre-commit: skip the git pre-commit hooks by adding the following flags to your commit:
pre-commit:
pre-commit: git commit -n (or --no-verify)
pre-commit:
pre-commit: This is ill-advised since the commit is broken.
pre-commit:
As did simply piping standard
into snazzy
.
@jsumners Good to hear. I'm seriously considering removing the ability to run snazzy
directly anyway, since in addition to this issue there are other issues it has caused. Specifically, this one recently: https://github.com/feross/snazzy/issues/16
Here's the PR that removes the ability to run snazzy
directly: https://github.com/feross/snazzy/pull/17
I'm not sure if this is a bug with
pre-commit
, a bug withsnazzy
or both. So I'm filing it here and copying @ferossThe following "project" will commit without error despite failing the linting step:
package.json
index.js
If you replace
"lint": "snazzy"
with"lint": "standard"
then the commit will fail as it should.