Discussion about this post

User's avatar
Mark Y's avatar

Hmm… this seems to take as a premise that the main point of the code review is to suggest changes to the code before it is suitable for merging.

I don’t think that’s the main benefit. If you’re worried about “inventing tasks for each other” then you can just have a rule that “if anyone makes a suggestion during code review that you disagree with, it suffices to say ‘no’ and if they feel strongly about it then they can file an issue in the tracker”

Okay, so what benefits do I claim still exist? In some sense the benefit is accidental: by mandating code reviews, you ensure that every piece of code has been seen by at least two people: author and reviewer. This helps people learn about the code base far faster than if they only ever look at code they wrote themselves, and reduces bus factor (Alice is on vacation, no one else knows this code). The record of the discussion can help archeologists later when trying to understand “why is it done this way???” and the discussion itself (even if the record is lost) means at least one other developer now understands the business requirement this bit of code was meant to address.

I’ve never been a manager but I suppose most of these benefits would be nearly invisible: YOU certainly know what the business requirement was; the benefits from 2 developers on a team of 30 understanding it, instead of just 1 in 30, are presumably very subtle. Perhaps they are too small to be worth it.

But if that’s what you think then you should say so; instead you never mention the knowledge spreading aspects at all, which makes me suspect you didn’t even consider it.

Thoughts?

Expand full comment
3 more comments...

No posts