DEV Community

loading...

Discussion on: Tell me an unpopular software opinion

Collapse
loujaybee profile image
Lou — Cloud Engineer

Code Review is a terrible reactive concept that is overly relied on by teams and used as a crutch for bad communication. Pair Programming is significantly more effective at spreading ideas, communication and critiquing code.

Collapse
adam_cyclones profile image
Collapse
aaronktberry profile image
Aaron Berry

I agree. I always found it daunting looking at a PR with 40+ files changed without even knowing what the PR should be doing. In the end, it always seemed to just end up being more efficient sitting down with the owner of the PR and them walking through what changed and reviewing it with them.

Oh and also actually getting the person to RUN the code to see it actually is doing what they expect it to do rather than assuming its working correctly.

Collapse
jessekphillips profile image
Jesse Phillips

Plug

Or a little more specific

Code review should include reviewing the history, it is such an important log, those in person conversations need written down for the poor maintenance folks when both of you aren't available.

Collapse
patryktech profile image
Patryk

I always found it daunting looking at a PR with 40+ files changed without even knowing what the PR should be doing.

Do you even CI, bro? :D

I'd consider looking into Continuous Integration. Smaller commits are oftentimes safer commits.

Collapse
daniel13rady profile image
Daniel Brady • Edited

I agree with your thought on pair programming, but I haven't experienced your view on code reviews. Are you saying that pairing can be a viable substitute for retroactive review?

Unfortunately pairing doesn't happen much in my current environment, and I don't do much to encourage it. In its absence, I think code reviews are better than nothing, but reviewing code effectively is just as challenging as delivering constructive feedback (because that's exactly what it is) and there aren't nearly as many devs who are good at this as there ought to be.

Collapse
loujaybee profile image
Lou — Cloud Engineer • Edited

Hey Daniel! :)

I find that often by the time code is up for review it's "too late" to make important changes. As you're wrestling with factors like the sunk cost fallacy and the de-motivating effects of suggesting that a colleague re-do their work seemingly to appease you.

Not only is it often "too late" to make changes but sometimes it's also hard to convey larger feedback via text. Complex changes are often only realistically conveyed in person going through piece by piece. Naturally over time though the amount of "explaining" is asymptotic as you get more and more inline with each others viewpoints.

My take is not that code review is bad (it's important to stress this point) it's just that I see code review is used as a crutch for not having adequate conversations prior and during the creation of code (since it's reactive in nature). And pair programming in my opinion is superior at type of communication.

However I know that my opinion is unpopular since I'd say the majority of developers I've worked with would admit that they dislike pair programming, which seems to tally your experience too so until the day comes that the industry invites more pairing I'll be over here shouting into the void! 😂

Thread Thread
daniel13rady profile image
Daniel Brady • Edited

Thanks for elaborating!

Honestly I quite enjoy pairing, but I've recently found myself in a remote work position that is also separated by about 8hrs from my teammates, which makes any sort of synchronous communication difficult. I've been experimenting with opening up a PR as soon as I make a first commit to a branch, and tagging my teammates with the hopes of exchanging feedback "early and often." I've met with mixed success, depending on how available my teammates are.

Thread Thread
nickdrouin profile image
Nick Drouin

For large stories, I try to push my team's / teammates to use a two-phased PR: one for the interfaces/design, a second for the code.
This promotes discussion on the approach early on.