3 ms·

Spot on. And why code reviews are at least 70% a waste of time. (Note that I left 30% for some of them to do some good, occasionally.)

hot_gril 4y ago
Code reviews are useful when the reviewers don't have OCD, which is only about half the time.
- Gigachad 4y ago
  All OCD and preferences should be encoded in the linter/formatter or a wiki page. It's obviously a waste of time to argue menial preferences in review.
  - hot_gril 4y ago
    Some of the documented preferences are annoying too, but at least you can adhere to them without going back and forth with a reviewer. What builds friendships is when you review each other's code in a way that ignores the parts of the style guide you both consider silly.
- AlbertCory 4y ago
  The xkcd cartoon hits on an orthogonal dimension, almost universally ignored, which is why I liked it: It's an economic endeavor, not an artistic one. I've been in some choruses, and it's really worth it to make every single thing as good as you can possibly make it. The audience for the concert appreciates the totality of what you do. A coding project is not art like that. It's not worth making most things perfect, and moreover, you can't tell now which ones are going to be worth it. Every hour you spend on polishing something that's going to be thrown away in two years is an hour you can't be doing something more valuable. The Net Present Value of extra work five years from now is almost zero.
  - hot_gril 4y ago
    Well it's the same dimension, obsessive code review wastes time and money.
Gigachad 4y ago
I've generally found pair programming to be more useful than code reviews. It's too hard to drop in at the end of someones weeks of work and try to meaningfully review it. And a lot of suggestions you might have would have been useful at the start of the work but aren't worth going back to fix. If you actually sit along and see the whole process you can actually understand the whole piece of work as well as give suggestions when they are useful and much cheaper to action.
- rqtwteye 4y ago
  At least reviews should be pair reviews where you talk to the author and can ask questions.
- mattbessey 4y ago
  > at the end of someones weeks of work That sounds like more of a criticism of large PRs and long feedback cycles than of code review itself.
dcow 4y ago
The goal of a code review should be to establish shared ownership. If that is valuable to your engineering org or team, then code reviews are valuable. Often teams don't have the time or discipline to treat reviews that way, in which case they are not valuable (or negatively valuable because they waste engineering bandwidth). A practice that I've seen work well is for the engineer writing the code or the team in general to have enough situational awareness to realize when something really would benefit from having immediate shared ownership vs when something doesn't and the responsibility falling on the code author or otherwise team to determine which pieces of work should be more thoroughly reviewed.
- AnimalMuppet 4y ago
  One of the goals of a code review should be to establish shared ownership (or at least shared-semi-competence). But code reviews have at least one other way of contributing value. They help find (moderately) obvious bugs, unsafe code, and unclear code. There is value in keeping blatant stupidity out of your codebase.
  - dcow 4y ago
    Those are 2nd order benefits derived from establishing shared ownership.
    - AnimalMuppet 4y ago
      That statement is false. If I call your code, I don't have to share ownership of your code to benefit from your code not having a bug in it.
      - dcow 4y ago
        That’s not what I said. I said you get the second order benefits by establishing shared ownership. Of course the qualities of the code are conferred to callers… that’s obvious.

hot_gril 4y ago

Code reviews are useful when the reviewers don't have OCD, which is only about half the time.

Gigachad 4y ago
All OCD and preferences should be encoded in the linter/formatter or a wiki page. It's obviously a waste of time to argue menial preferences in review.
- hot_gril 4y ago
  Some of the documented preferences are annoying too, but at least you can adhere to them without going back and forth with a reviewer. What builds friendships is when you review each other's code in a way that ignores the parts of the style guide you both consider silly.
AlbertCory 4y ago
The xkcd cartoon hits on an orthogonal dimension, almost universally ignored, which is why I liked it: It's an economic endeavor, not an artistic one. I've been in some choruses, and it's really worth it to make every single thing as good as you can possibly make it. The audience for the concert appreciates the totality of what you do. A coding project is not art like that. It's not worth making most things perfect, and moreover, you can't tell now which ones are going to be worth it. Every hour you spend on polishing something that's going to be thrown away in two years is an hour you can't be doing something more valuable. The Net Present Value of extra work five years from now is almost zero.
- hot_gril 4y ago
  Well it's the same dimension, obsessive code review wastes time and money.

Gigachad 4y ago

I've generally found pair programming to be more useful than code reviews. It's too hard to drop in at the end of someones weeks of work and try to meaningfully review it. And a lot of suggestions you might have would have been useful at the start of the work but aren't worth going back to fix. If you actually sit along and see the whole process you can actually understand the whole piece of work as well as give suggestions when they are useful and much cheaper to action.

rqtwteye 4y ago
At least reviews should be pair reviews where you talk to the author and can ask questions.
mattbessey 4y ago
> at the end of someones weeks of work That sounds like more of a criticism of large PRs and long feedback cycles than of code review itself.

dcow 4y ago

The goal of a code review should be to establish shared ownership. If that is valuable to your engineering org or team, then code reviews are valuable. Often teams don't have the time or discipline to treat reviews that way, in which case they are not valuable (or negatively valuable because they waste engineering bandwidth). A practice that I've seen work well is for the engineer writing the code or the team in general to have enough situational awareness to realize when something really would benefit from having immediate shared ownership vs when something doesn't and the responsibility falling on the code author or otherwise team to determine which pieces of work should be more thoroughly reviewed.

AnimalMuppet 4y ago
One of the goals of a code review should be to establish shared ownership (or at least shared-semi-competence). But code reviews have at least one other way of contributing value. They help find (moderately) obvious bugs, unsafe code, and unclear code. There is value in keeping blatant stupidity out of your codebase.
- dcow 4y ago
  Those are 2nd order benefits derived from establishing shared ownership.
  - AnimalMuppet 4y ago
    That statement is false. If I call your code, I don't have to share ownership of your code to benefit from your code not having a bug in it.
    - dcow 4y ago
      That’s not what I said. I said you get the second order benefits by establishing shared ownership. Of course the qualities of the code are conferred to callers… that’s obvious.