firtoz / react-three-renderer

Render into a three.js canvas using React.
https://toxicfork.github.com/react-three-renderer-example/
MIT License
1.49k stars 155 forks source link

Commits take too long because of pre-commit hook #167

Closed toxicFork closed 7 years ago

toxicFork commented 7 years ago

This makes contribution and work more difficult, it's not the end of the world to have some bad styling that could be resolved eventually.

Need to either make eslint run faster, or remove the pre-commit hook

toxicFork commented 7 years ago

Alright, after a few minutes of thinking about how to accelerate it, I have decided to remove the pre-commit hook.

Could check to speedup eslint itself if necessary (see comments like this), but in the end if eslint fails our friend Travis will get angry anyway.

johnrees commented 7 years ago

Ah! That was a fast decision, I'd just started working on a branch to address this by only linting files that have changed

I was planning to use something like exec('git diff --cached --name-only --diff-filter=ACM', function (err, stdout, stderr) {}); to get the list of files that needed to be processed

toxicFork commented 7 years ago

Thanks @johnrees :D

First of all, we think similarly!

I had first started by installing husky then I looked at which parts are slow e.g. -"do we really need to use gulp?" -"well ok let's try running eslint from cli" -"that's slow too..." -"welp..."

Linting only diffed files (and even changed lines?) is a good idea and should be much faster indeed!

But in the end I think it may not be worth my or your time after comparing pros/cons...

johnrees commented 7 years ago

@toxicFork I think the long-term solution, that seems to address this exact problem, would be to use lint-staged

And maybe(?) switch to prettier for linting too.

https://github.com/prettier/prettier#pre-commit-hook-for-changed-files

johnrees commented 7 years ago

Something like this for example. Means you won't need to run gulp at all.

https://github.com/toxicFork/react-three-renderer/compare/master...johnrees:precommit?expand=1 might work for now

toxicFork commented 7 years ago

Thanks for the investigations, please open the PR and I will check to see if it passes my "this is fast enough to not be too annoying" tests :D

But TBH latest few non-hooked commits were so fast that I felt like ⚡️!