Automatically reject buggy pushes with git hooks. | Hacker News Reader

Automatically reject buggy pushes with git hooks.

stavros.io

21 pointsstavros10 comments

Make sure that you trust your sources before putting something like this in your workflow. I'd guess that these scripts are easily exploitable. A suitably malformed patch gives an attacker a remote code execution on your machine!

Obviously not a worry if you know your sources or your project isn't open to all. It's just that with something like this set up, it would be tempting to use it to pre-filter changes before you do a 'proper' review of the code, and by that time it's too late...

Automatically frustrate and annoy your contributors! Make people never want to work with you!

PS. Don't do this, ensuring test success is one thing. Requiring PEP8 validation is dumb.

Because the codebase shouldn't have one consistent style?

A contributor is supposed to be following the coding guidelines of the project from the start. If they don't bother to do that, they absolutely should be rejected. Why is it hard to follow a consistent style?

Because just remembering all of PEP8's requirements (which are mostly unjustified) is a frustrating burden. Keeping a consistent style is one thing. Mandating adherence to PEP8 is way over the top.

Ideally anything that you reject for should also be conveniently baked into the build system so that you can do things like `make validate` before pushing to save yourself the embarrassment.

If you use Gerrit for code reviews, you can also have Jenkins add reviews to commits in Gerrit with the Gerrit Trigger:

https://wiki.jenkins-ci.org/display/JENKINS/Gerrit+Trigger

One of the components of the Dokku project[1] is a component named gitreceive[2]. It creates bare git repositories on the fly when you push to your git server, and then runs a general receiver script.

[1]: https://github.com/progrium/dokku

[2]: https://github.com/progrium/gitreceive

I feel like pre commit is better than pre receive.

pre-commit would be a bad idea. You would have to ask all of your developers to install the hook, instead of just installing it on your 'official' repos and it would quickly become a nuisance to developers who liked to create temporary commits with no intention of ever giving them out to the rest of the world. These developers would quickly become annoyed at the pre-commit hook, turn it off, then you would be back where you started (except now you would have a false sense of security).

I do agree that pre-commit isn't the best choice though. I prefer update hooks; I find they are simpler to write and easier to read.

Reply on news.ycombinator.com