3 ms·
I suspect this may not be too well received, but I've thus far managed to avoid regular formal reviews, and I'd advise others to think twice before imposing the
by matfil 8y ago
I suspect this may not be too well received, but I've thus far managed to avoid regular formal reviews, and I'd advise others to think twice before imposing them. They seem like a big step towards treating programmers like cogs in a machine rather than competent individuals, and for me that's a direction I don't want to be headed.
- ccharles 8y agoHave you tried doing them? Other users have already described healthy code review processes that improve code quality while helping all participants to grow individually and as a team. How does review treat programmers like cogs in a machine?
- matfil 8y agoYes, I've encountered them. It seems really hard to do them in a way that doesn't touch heavily on style. To the extent that the preferred solution to trying to shift the focus back to deeper issues is to go one step further and mandate auto-formatting tools. So individuality-of-style has been dismissed out of hand. Can they be helpful? In some cases, perhaps yes. But they're definitely pushing a collective-ownership, try-to-smooth-away-individual-differences agenda.
- ccharles 8y agoIt is much easier to read code that is written consistently, and it's much easier to contribute to code when a style guide is available. This isn't "pushing an agenda". And what's wrong with collective ownership? Code review shouldn't involve bikeshedding about whether braces should go on the next line or not. It should be about design decisions, clarity of expression, algorithm complexity, etc.
- geofft 8y agoI 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.
- matfil 8y agoI 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".
- throwaway84742 8y agoAnd 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.
- bostik 8y agoWe 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.
- throwaway84742 8y agoTBH 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
- joshuamorton 8y agoSeconding Geoff, I find code review to be one of the best forms of mentorship possible for a junior to mid engineer.