plexus / yaks

Ruby library for building hypermedia APIs
http://rubygems.org/gems/yaks
MIT License
236 stars 26 forks source link

Using pre commit hook for rubocop #89

Closed groyoh closed 9 years ago

groyoh commented 9 years ago

I sometimes tend to forget to use rubocop before pushing and I usually find out via travis that the rubocop task failed. I think it would nice to have a pre commit hook (using a tool like overcommit) to verify rubocop errors before commiting. This would keep pull request clean from rubocop errors. Overcommit could also be used for other hooks and tasks (json file lint, running specs before pushing,...). I already have a PR ready for this but I wanted to see what you guys think about it first.

plexus commented 9 years ago

I wasn't aware there existed a "git hook manager" :) I was thinking as well to stick rubocop in a hook (pre-push). this is a very straightforward thing that each dev can do for themselves. Overcommit at this stage seems overkill to me.

This would be a good thing to mention/suggest in the developer docs (which I promise will exist at some point)

plexus commented 9 years ago

Also if there are specific things you routinely run into, it might be that we're being too restrictive. I'm open to relaxing the rules.

groyoh commented 9 years ago

It is not really about the rules. It's just that sometimes you may forget a thing or two and commit without thinking about it. Why do you think it is overkill?

plexus commented 9 years ago

I don't see why we need a "hook manager" just to add a single one line hook.

echo bundle exec rake rubocop >. git/hooks/pre-push

also works

groyoh commented 9 years ago

But that would not be shared among contributors and thus would need to be done by every developer on every machine he/she uses. I admit that's not a lot of work but new contributors might now necessarily know about it. But as you said, adding a section about it in the developers docs might the solution to this.

plexus commented 9 years ago

You have to do that anyway, hooks always have to be installed locally. Overcommit just adds a layer of indirection. If a new contributor doesn't overcommit --install in their newly cloned repo they also won't get any hooks, or am I grossly misunderstanding things?

In one scenario you just have to learn about git hooks, in the other you have to learn about git hooks and about overcommit. In both cases you need to run a single command to get the hook working.

groyoh commented 9 years ago

My bad! I thought overcommit automatically added the hooks somehow. Then you are totally right!