Code Review Is Not About Catching Bugs (www.davidpoll.com)
from codeinabox@programming.dev to programming@programming.dev on 23 Mar 15:29
https://programming.dev/post/47658920

Code review answers: “Should this be part of my product?”

That’s a judgement call, and it’s a fundamentally different question than “does it work.” Does this approach fit our architecture? Does it introduce complexity we’ll regret in six months? Are we building toward the product we intend, or accumulating decisions that pull us sideways?

#programming

threaded - newest

MagicShel@lemmy.zip on 23 Mar 16:18 next collapse

If something gets to code review before this question is answered, I can’t help but feel something has gone dramatically awry.

codeinabox@programming.dev on 23 Mar 16:33 collapse

I agree but it depends on how teams create and refine their tickets. For example, you could have high level tickets, and someone picks one up and creates an implementation that’s not an appropriate fit for your architecture.

org@lemmy.org on 23 Mar 16:42 collapse

Brass said they want a feature by the end of the week. So, make it happen—regardless of your opinion.

litchralee@sh.itjust.works on 23 Mar 16:32 collapse

The only way I’m able to reconcile the author’s title and article to any applicability to software engineers (ostensibly the primary audience in this community) is to assume that the author wants software engineers to be involved further “upstream” of the software product development process.

Code review answers: “Should this be part of my product?” That’s a judgment call, and it’s a fundamentally different question than “does it work.”

No, but yes. Against the assertion from the title, bug-finding is very much a potential answer to “does this bug belong in the codebase?”. After all, some bugs aren’t bugs; they’re features! Snide remarks aside, I’m not sure that a code review is the time to be making broader choices about product architecture or market viability. Those should already have been done-and-settled a good while ago.

Do software engineers make zero judgement calls? Quite the opposite! Engineers are tasked with pulling out the right tool from the toolbox to achieve the given objective. Exactly how and which tools are used is precisely a judgement call: the benefit of experience and wisdom will lean towards certain tools and away from others. But a different group of engineers with different experiences may choose differently. Such judgement calls are made in the here-and-now, and I’m not exactly keen on going back in time to berate engineers for not using tech that didn’t yet exist for them.

If the author is asking for engineer involvement earlier, well before a code review, then that’s admirable and does in-fact happen. That’s what software architects spend their time doing, in constant (and sometimes acrimonious) negotiation with non-engineering staff such as the marketing/sales team.

That said, some architectural problems only become apparent when the rubber meets the road, when the broader team is engaged to implement the design. And if a problem is found during their draft work or during code review, that’s precisely the right time to have found that issue, given the process described above where the architects settle on the design in advance.

If that outcome is not desirable, as the author indicates, then it’s the process that must change. And I agree in that regard. But does that necessarily change the objective of what “code review” means? I don’t think so, because the process change would be adding architectural review ahead of implementation.

If we’re splitting hairs about whether a broad “review” procedure does or doesn’t include “review of code”, then that’s a terminological spat. But ultimately, any product can only be as good as its process allows. See aviation for examples of excellent process that makes flying as safe as it is.

Making the process better is obviously a positive, but it’s counterbalanced by the cost to do so, the overhead, and whether it’s worthwhile for the given product. Again, see aviation for where procedural hurdles do in-fact prevent certain experimental innovations from ever existing, but also some fatal scenarios that fortunately no longer happen.

In closing, I’m not entirely sure what the author wants to change. A rebrand for “code reviews”? Just doing something different so that it feels like we’re “meeting the crisis” that is AI? That’s not exactly what I would do to address the conondrums presented by the rapid, near-uncontrolled adoption of LLMs.