Hacker News Reader

Comment by geofft | Hacker News Reader

I find that good programmers will voluntarily request reviews whether or not they are required by process, and bad programmers will attempt to minimize them even when required by process. Having a policy is a good way to weed out the bad people, and won't affect the good people.

View on HN

I might agree with you in the sense that sometimes there's a worthwhile discussion to be had and of course in such cases it's good to talk to someone who's opinion you trust.

That's different from mandatory code review, which almost inevitably will end up covering style and process considerations.

It's certainly a filter, but depending on your perspective I'd be a little cautious about "won't affect the good people".

And moreover, I send my gnarliest PRs to the most vicious reviewer on the team, just because 3 months from now finding out what’s wrong is exponentially more difficult. I want to get it right the first time around, and I want other folks to be able to make changes anywhere in the codebase, even in the gnarly parts.

We have mandatory code reviews for everything, which encourages small, bite-size commits and frequent merges. Majority of the changes goes through with just one other person taking a look.

But we also specify (and highly encourage) that anyone trying to land a more invasive or complex change should require multiple review approvals. The most demanding "please pile on, we want eyes on this" request I remember was set to require 4 separate approvals.

TBH I favor atomicity over small size. So we get a few gnarly PRs a week. That’s also why we use Reviewable instead of GitHub reviews

Reply on news.ycombinator.com