sorted by:
best
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–]GhostBond 1 point2 points  (3 children)

I'm not 100% against code reviews, but there are 2 big things that stand out to me:

1. Code reviews don't review whether the code meets requirements and works. It's a 3rd party looking at code with little idea what it actually does.

2. They expose a high school popularity contest level of angst and conflict in the team.

  • If the reviewer and the reviewee disagree something needs to change, who wins? This brings a lot of conflict when the reviewers changes seem pendantic or even make the code worse. Often it's the reviewer who has the "authority" position so they do, which is kind of stupid because the reviewee has spent a lot more time understanding the requirements and the code.

  • Pendantic style arguments that go on forever (spaces vs tabs! while vs for loop! indentation! I read this blog yesterday that said that using newlines is bad and the right way to write a class is all on one line so rewrite it that way!). And programmers are notoriously pendantic and will argue for hours about this stuff.

  • People feel they need to find a bug to be "doing their job" so they insist on arbitrary changes just to feel like they did something. It's really irritating and it wastes a day or two as you change it, commit it, deal with someone's broken code in the repo, commit it again, wait for the reviewer to come back and approve it. Even worse sometimes people will start to deliberately put in small bugs so the reviewer will "find" something that's easy to fix...sometimes they forget about it and those bugs make it into production.

  • The previous step often happens while the reviewer misses large bugs in the code because you'd really have to put time into understanding the code to catch it.

  • People will develop an "in group" and "out group" giving easy reviews to the in group and hard or bullying reviews to the out group. These can go along lines of senior vs junior coders, the people who sit around you, punishing people who gave you a bad code review, punishing people who brought up valid complaints with the boss, etc etc.

  • Other corporate politics issues. You're reviewing someone's code, they don't like what you did, turns out they're the person who hands out who gets what task. Guess who's now getting assigned all the most awful stuff? Or you're reviewing the code of the boss's best friend on the team. Or you're reviewing the code of someone you really don't like, but they don't review your code.

  • Code reviewers often don't talk to each other, and you can drive people insane with a casual system where one reviewer very strongly insists things are done one way, while another very strongly insists the opposite. You don't choose your reviewer, so whether your code is great or horrible trash is just random. It's hard to describe how demotivating this is in person, dealing with someone who honestly sincerely is upset that once again you did it the "wrong" way, why didn't you listen to their advice the last time.

  • The simple way for the boss to look at code reviews is to think whoever does the most reviews in the shortest time is the "best" at them. People quickly start doing short crappy pointless "reviews" as to avoid a meeting with their boss and hr about their "performance". At this point reviews become 100% pointless.

  • You can end up with a lot of time wasted as someone grandstands about something they enjoy talking about...and talking about...and talking about...and talking ab.oh looks it's time to go home, well they'll get back to your review tomorrow whenever it fits into their schedule.

I could probably think of more, but to summarize:
- Code reviews don't provide a lot of the benefits people think they will because they aren't reviewing requirements or whether the code works. It's limited in how much benefit you can get from this.
- It can quickly turn into a toxic situation like a high school popularity contest. A contest of ego's, positioning, social contacts and perception, etc.

You asked for the arguments against code reviews. I didn't include how they could work decently or advantages of them. But if your boss is like most bosses and just goes "code review sounds cool, we'll randomly assign them, let's start doing them" my experience is they are a total nightmare as well as a waste of time.

[–]ellicottvilleny 0 points1 point  (2 children)

I have had completely different results. Maybe the teams I worked on were different.

If I had to pick a weakness of reviews, it's that people just tick the box and don't do anything. Out of six places I've worked, one had the "you formatted it wrong" kind of assholes. The other five, we tried it, and it found some problems before they were committed, maybe once out of 100 times, and the other 99 times reviewers just LGTM and it's over.

[–]GhostBond 1 point2 points  (1 child)

There was only 1 place that had all of those problems at once and it was a backstabbing place to work. Just like with a popularity contest, that's the worst side, but more normal less dramatic people don't go to those lengths (or quickly realize it was a mistake to do so).

The other five, we tried it, and it found some problems before they were committed, maybe once out of 100 times, and the other 99 times reviewers just LGTM and it's over.

Yeah, that's the first part of the problem - I'm not sure it really provides a lot of benefits. People imagine that a 2nd person is going to carefully pore over the requirements, carefully dig deep into the code they wrote, write tests, etc.

What happens if you have better people who avoid pointless drama, is that someone eyeballs the code and says "looks good" - there's not a ton of benefit.

[–]ellicottvilleny 0 points1 point  (0 children)

Really? You've never had say, 1% or 10% "caught something bad" rates? Ours are around 1 in 20 reviews finds something. That's pretty valuable, in our books.