5 ms·
True. But having just one reviewer can be bad too: when two people review each other often, they often "collude" and start to rubber-stamp shitty code. Or the o
by dunkelheit 9y ago
True. But having just one reviewer can be bad too: when two people review each other often, they often "collude" and start to rubber-stamp shitty code. Or the other extreme - they can go into an argument over some issue with no way to break the stalemate.
Two reviewers seems optimal if you can afford it. Possibly with one person doing the bulk of the reviewing and the other more in the role of providing oversight and breaking ties if they occur.
- greglindahl 9y agoIf you have people submitting shitty code for review, you've got bigger problems than needing a better review system.
- bennofs 9y agoWelcome to open source maintainership
- greglindahl 9y agoIf you have new folks who aren't fully socialized to the project yet, having multiple reviewers is much worse than having a couple of people specifically individually coaching new would-be committers. I use a coaching approach for new hires at my startups.
- aurbano 9y agoCould you describe your coaching approach? That sounds interesting, and is often a difficult problem to solve
- greglindahl 9y agoWell, the basic idea for coaching a brand-new employee is that you hired a perfectly smart person for the job, but your company has a strong coding culture with a particular style that the new engineer isn't familiar with. So it's a big part of on-boarding to not only teach this smart engineer the crazy weirdnesses of your system, but also teach them (what they will perceive as) the oddities of your corporate coding style. It can be good to do this outside of the public eye, so the initial cycle of "wtf, why is this considered standard" followed by "well, it's not so much that this is the best way, but it's more likely that we'll be able to understand each other's code if we all do it one way" can be done in private. Once the engineer is well-versed in the system, they are unleashed to quietly subvert it, if that is their will :-)
- koolba 9y agoMaintaining a popular open source project is a great way to learn to politely say “fuck you this is crap”. It’s important to be consistent with your online voice though. If you’re always blunt and terse then, generally, people won’t take it badly. But if you slide between extraneous gestures and out right profanity then eventually someone will take it personally.
- dunkelheit 9y agoHere are some of the reasons people will submit shitty code: they (even subconsciosly) think it will be somebody else's problem; they are junior; they don't fully understand the system they are changing; they don't feel polishing the code is valued. Many of these problems can be improved with a good review system. I don't want to enter the people vs. systems debate but certainly a good system can help.
- viraptor 9y agoThere are also 2 cases of tunnel vision that I started noticing in reviews. 1. You've been working on some non-trivial change for long enough that you find some element of it trivial and not worth documenting, but others have no idea why it's there. 2. You've been looking at something so long that a "temporary code, I'll fix it later" starts looking natural and it takes someone else to call it out.
- table-delete 9y ago> you've got bigger problems than needing a better review system I'll set aside the fallacious premise that everybody can stack their team with 10x engineers. Some of us actually work with interns and juniors. That being said, your comment doesn't invalidate the need for a better review system. Software quality isn't defined by any one tool or process; the product is the end result of a chain of tools and processes...
- wumpus 9y agoI'm all for good review systems. But any review system that labels a large fraction of the submitted code as very deficient indicates that the solution is to act before review. And I've coded with (and hired) interns and juniors, so it's not just a selection effect. In fact, the worst code I've seen came from a senior engineer, with strong opinions, who wrote the world's most blecherous code. We fired him, but not soon enough.
- scotu 9y agoI think 1 reviewer works better in general, but multiple reviewers could be made to work if the reviews and the reviewers were separate and unaware of the other?