Adds lint and test pre-commit hooks#2077
Conversation
|
@mcpolandc thanks! good idea. Thoughts on using |
|
Hi @JedWatson! Yeah, running the linter on only the staged files when committing makes a lot of sense. I will make changes to this effect 😄 Just extending on this... I wonder if we should maybe add the test cover check at the commit stage as well. Maybe |
a481154 to
207ca7c
Compare
|
Changed this to run lint (only staged js files) and test scripts on commit rather than push. |
package.json
Outdated
| ] | ||
| ], | ||
| "lint-staged": { | ||
| "*.js": ["eslint", "git add"] |
There was a problem hiding this comment.
What is the point of git add here? unless eslint is being run with the --fix flag (which it's not and I'm happy to leave it that way) this wouldn't be necessary; and it has the side-effect of preventing you from committing only staged chunks of a file without making it an obvious thing...
Surely just eslint on its own would be sufficient (and safer!) here?
There was a problem hiding this comment.
I knew that I didn't want to have it fix linter errors automatically but I didn't really understand what git add was doing. Reading further down the readme of lint-staged helped with that 😛. I totally agree with what you're saying here so making the change.
|
Thanks @mcpolandc! Left a note there for feedback but otherwise this looks great! |
207ca7c to
cdedc32
Compare
|
Great, thanks! |
It occurred to me while reading the contribution guide that the project might benefit from automatic push hooks to lint and test.