git hooks lint working directory instead of staged changes

[ckeeney]

Right now the git hooks that are run lint the current working directory, not the currently staged changes. This causes 2 problems.

1. I may have changes I want to commit, and also have some changes that are not ready to commit. I cannot commit my ready changes unless the files I'm not even committing also pass lint.

2. The second problem is more serious. Suppose you have a single file with changes you want to commit. You run git add file.js; git commit and see some lint errors. Now you quickly fix those errors and git commit again. This time, it lets you commit, but you didn't even stage the changes fixing the lint. Now your non-linted changes have been committed to the repo.

[jamonholmgren]

@ckeeney That's a valid complaint. I'm actually in favor of removing the git hooks entirely. @GantMan @skellock thoughts?

[GantMan]

I don't think it's a good idea to remove the hooks, but I think they should be on push not on commit. Not sure who put them on each commit, but that was quite bold.
RE: issues at hand

1. You can bypass a check with the flag --no-verify so when you want to bypass the hook, you're fine!
2. Running git add file.js; git commit is definitely an interesting way to make a commit add dependency. But the mistake made in running git commit with no staged changes can be easily rectified upon noticing with a simple git add file.js; git commit --amend and then all is right in the world.

I'd be satisfied moving them to a plugin, but I still feel they are best practice. I'm very opposed to removing them completely.

[ckeeney]

I'm not really in favor of removing the git hooks completely. I also don't mind them running on git commit. If they only run on git push, then there are already unlinted changes committed to the repo.

I don't understand what you mean by

Running git add file.js; git commit is definitely an interesting way to make a commit add dependency. But the mistake made in running git commit with no staged changes can be easily rectified...

I don't typically run those commands in one line like that, but know people who do. My point wasn't about running git commit with no staged changes. In my hypothetical, file.js exists and has linting errors. You git add file.js and git commit. The git hooks run showing the linting errors. Suppose now that you correct the errors and run git commit again. Now the lint will pass and the commit goes through. However, the fixes for the lint are still unstaged and uncommitted.

[skellock]

@GantMan:

Not sure who put them on each commit, but that was quite bold.

I did because it was there before. I wouldn't call it bold though.

@ckeeney:

What I like do on my projects is:

1. lint & format the staged files on precommit
2. compile & test the whole project on prepush

I recommend using lint-staged. It's excellent.

Lemme know if you wanna see an example.

@jamonholmgren:

Didn't wanna leave you out. Hey dude!

[ckeeney]

@skellock I think linting and formatting staged files only is the right way to go here.

I personally prefer to not compile or test on prepush because I push to a CI server that does it anyways. However, I agree that compiling and testing on prepush is a reasonable default.

Honestly I wasn't too interested in fixing it in just my project. It's only a minor annoyance with many work-arounds. I was just documenting it so it can hopefully be fixed in Ignite eventually.

[GantMan]

@skellock BOLD! BOLD BOLD BOLD BOLD BOLD!

In all seriousness I think it depends on the shop, and it depends how long your tests take. In some projects we're seeing 30seconds of tests/lint. Not common, but at some point it makes a simple commit throw you off your timing.

In any case, def want to keep the commit hooks all around, just need to tweak them. 👍

[skellock]

I like my git hooks like I like my coffee. Bold and heartburn inducing.

[ryanlntn]

Git hooks have been removed per this conversation: infinitered/ignite-andross#120

Relevant commits:
Andross - infinitered/ignite-andross@c2aaac1
Bowser - infinitered/ignite-bowser@f97898d
ignite-standard - robinheinze/ignite-standard@72253cf