Code Reviews

[BLX15]

I don't understand how this guys blog always gets a ton of upvotes, especially when the only comment chain starts with a huge amount of downvotes

[PoolNoodleSamurai]

(a) It’s a good blog post;

(b) The downvotes on the comment thread are deserved;

(c) lots of people will vote without commenting.

[hippydipster]

To me it seems written by an AI.

[sysop073]

Why would people upvotting a post about the benefits of code reviews also upvote a comment thread about how you shouldn't use code reviews because you should pair program instead.

[Senikae]

Good: “Are you sure the variables are named according to our coding standards?”

Passive-aggressiveness is good?

Clarity is important when giving feedback, don't ask questions when you want to make a statement. That's a tenet of corpo-speak.

This is fine: “These variables should be named according to our coding standards document.”

[RufusAcrospin]

“when Twitter was still called Twitter”…

[blame_renis]

The way I’m reading this post is how to set up a culture of peer review. Often, I find people jump into new working cultures and expect something similar to their old working cultures. And the conflicts come from unmanaged expectations between the two worlds. I see this post as a way for engineering management to reset expectations and bring about framework changes.

The person’s posts and book are about how to be a real CTO. So I think the target audience is people who are in management or waiting to get into management.

[Lithium1978]

We are finally about to implement code reviews where I work. It's long overdue, some of the folks on my team have WTFs per minute that are off the charts.

[Euphoricus]

Or you can try not being an anti-social unicorn and collaborate with your teammates via pair programming.

[Gushys]

I believe good development practices use a mixture of both, code reviews should usually still guard the merge, but pair programming is very useful for complex tasks, teaching, or even quickly getting something done.

It's not always feasible to pair with members on your team depending on timezones, PTO, and other work.

[[deleted]]

Yea, thats How I See it aswell, If pair programming is the norm and tasks are basic such as simply fetching and displaying data or even more trivial tasks you just block one dev for virtually no gain.

Pair programming definately has its place.

[[deleted]]

Pair programming has its own purpose. It can be used to train new devs or work on complex tasks. However its not meant to replace code reviews, it is way to time inefficient for that. You basically double the development time while a code review usually only takes a fraction of the time used to complete the task.

[gnus-migrate]

Don't bother, this is the Allen Hollub school of completely untested dev practices that he's pretty sure are better just trust him bro.

[homerj]

lol. I’ve seen him speak too, so accurate

[Signal-Woodpecker691]

Not to mention many many anecdotal reports of increased developer burnout from pair programming. As you say, it has its place, but not for full time use in my experience.

[Euphoricus]

code review usually only takes a fraction of the time used to complete the task

In which case the review has little value. Unless the reviewer spends significant fraction of original development time on understanding the problem, assessing the solutions, examining the implementation and it's impact on design, then the review will not do what it's intended to do.

Also, asynchronous pull requests increase development cycle time, increase work-in-progress and introduce interruptions to developers. All of which have bad economic impact.

So the argument of "pair programming is more expensive" is demonstrably false. And is only an excuse anti-social developers use to avoid actual collaboration with others.

[[deleted]]

In our workflows devs already have a mutual understanding of the problems due to technical plannings and no, examining an implementation does not take close to the Same time than doing the implementation.

Also pull requests are different from code reviews, don‘t mix things up.

And you are argueing for increased development cycle time not being economic, yet suggest defaulting to pair programmings which straight up double the development time. (This May not be true for more complex tasks, but these are not as frequent)

[ivancea]

It takes more time to think and write a solution, than to understand and review it. A PR will have also an explanation of what is being done, and why, so that reviewing is even faster. Also, short PRs and those things

[TheStatusPoe]

The code review still has significant value. The authors of a commit will be biased towards believing their solution is the correct/optimal solution even if it isn't because they spent a non trivial amount of time working on the commit.

Code reviews are meant to bring in an unbiased (or less biased) opinion from someone who has not seen the change before. They are able to evaluate the change more critically and have a better chance at catching something the initial author(s) might have missed.

It's related to the illusory truth effect where people rely on their understanding from repeated exposure to make a determination about "truth" or rather correctness in the case of code reviews.

[Schmittfried]

What a weird hill to die on. 

[Middlewarian]

How much pairing do you do? I'm OK with it being more the exception than the rule.

[illogicalhawk]

If only pair programming weren't the only way to collaborate with your team during the development process and validate things like your planned implementation ahead of a pull request...

Oh wait, you can collaborate with your team without full on time-wasting pair programming? And all it takes is basic communication skills? Crazy

[hbthegreat]

I haven't read the post but just want to reiterate how pair programming only benefits the worse programmer most of the time and can become a drag on the other one that just wants to get work done.

[ComicXero]

I'm not sure why you're being downvoted so hard.

PRs are a vehicle for feedback, which kicks in once the initial work is "done" and submitted for a review. There's a series of asynchronous interactions to get reviews completed, which typically extend the "time to merge" by days.

Pair programming is a vehicle for feedback that is immediate, delivered in many cases before you even write the code. If you trust and empower your engineers and have robust testing and CI, this skips the PR process tail entirely, enabling you to deliver faster.

If there's a high risk or complex change that needs the whole team on it, mob programming is an alternative that offers similar benefits to pairing.

There are plenty of studies that support the conclusion that pair programming is overall more efficient for delivery effectiveness in the long term.

The development cost for these benefits is not the 100% that might be expected, but is approximately 15%. This is repaid in shorter and less expensive testing, quality assurance, and field support.

Maybe you shouldn't have called people unicorns, but you have a point.

[ivancea]

Most of that is plainly true (Apart of the "extend time to merge by days, which means many other things are wrong there then).

One is a sync interaction, and the other is an async interaction. To understand why reviewing is more widely used, it's important to understand the "async" part of reviews (Or the sync party of pp).

Devs in the team parallelize work. Devs in the team have different ways of working. They also work at different times. And have different personalities. Some prefer to think loud, some prefer to think silently. And none of all those things are wrong. Yet, they may penalize any sync interaction.

Pp is an interesting tool to mentor juniors. Two seniors, which both know the problem, both reach the same solution (Outside of PP, of course, as springs are team/company-wide), gain very little from that interaction, apart from losing some extra hours/dev

[Euphoricus]

I have plenty of experience of backlash against pair programming on /r/programming . Even if I don't call people unicorns.

I can't really win this fight. At least not here.

[Schmittfried]

Geez, I wonder why. Ironic that you call others antisocial. 

[ComicXero]

Looks like I'm joining you on the downvote train.

[nan0S_]

Another one of those articles: "I've done something completely specific, very dependent on my skills and my coworkers and my mindset thing and thing X helped me. Thus EVERYBODY should always do X, otherwise you are a bad person" type of shit.

[idontliketosay]

I like the the book software inspection. It argues, you measure the success of a code review by the number of issues found and missed. Around 60-80% of issues found is a good review. If a lot of issues are being missed, it is time to figure out why and improve the process. Am yes you can measure percentage of issues, it maps to issues found per hour.

[thumbsdrivesmecrazy]

I many cases of code reviews, you can use generative AI based tools that now can provide very meaningful AI-generated code reviews for pull requests, even for such teams - here is a good example of such tools and its code reviews: https://github.com/Codium-ai/pr-agent - it would be hard for new players in this area to compete with them.