Queens-Hacks / qcumber-api

transforms qcumber data repo into something blah
1 stars 3 forks source link

add a lint command to verify conformace to the style spec #10

Closed uniphil closed 10 years ago

uniphil commented 10 years ago

I probably should have added this to Graham's branch for the pep8 pre-commit hook, but only thought of it after.

uniphil commented 10 years ago

noooooo your beautiful bash script! haha.

So... one cool thing about your bash script was that it (if I read it right) was only linting changed files*. Do you think it might be worth adding an argument to the lint command to check only staged changes so the pre-commit hook is faster?

* slight issue with the bash implementation was that it wasn't just linting staged things, but anything that changed. So an unstaged change to a file that passed linting could still be committed (which I did a couple times).

uniphil commented 10 years ago

I just force-pushed because I messed up my branch locally, so hopefully you didn't pull the commit called blah after 7a187d4. Otherwise it should be fine.

I added a timer to record how long linting takes, so we can keep an eye on if it starts taking too long, and maybe then look at linting staged files in the pre-commit hook or something. It's about 0.1s for me now, so I'm not worried.

Here's some food for thought on the pre-commit hook for checking staged vs changed files: http://tech.yipit.com/2011/11/16/183772396/

The script is pretty heavy duty. I'm not too worried about ours for now, since travis will only see whatever is actually committed anywya.

Graham42 commented 10 years ago

Cool, ya it's pretty much instantaneous for me too, I will look into it if it gets at all slow.