Live data from Hacker News

The primary purpose of code review is to find code that will be hard to maintain

mathstodon.xyz

101–110 of 193 posts

Re: The primary purpose of code review is to find code that will be hard to maintain

#102
post #44

No, the real reason for the code review is to protect the moat of senior engineers/leaders that would nitpick on minute details of code while ignoring the big picture to make sure they can gatekeep any promotions and their competition.

If that's how it functions where you work, I'd be looking for a new job.

There is a difference between how things are intended to be used and how they are used. Code review has been weaponized this way even at Google.

Re: The primary purpose of code review is to find code that will be hard to maintain

#103
This is the big thing I'm struggling with:

Code reviews are still a critical step in most workflows. Though seems like everyone uses them for a different purpose: extra pair of eyes to meet a regulation/security, style police, and what this one says: maintainability.

And at the same time, code reviews are now a massive bottleneck in the development pipeline. Frankly, in a lot of teams they have been that for a long time. Though many would argue it was the only thing stopping absolute crap going into production.

But in a world where none of us writes code anymore, and I think we're there, even if the future is "just not evenly distributed" (Gibson), why should we have to do code reviews, the worst job of all?

Leaves me thinking that code reviews will land in the dustbin of history.

But for now I don't have a better solution.

Re: The primary purpose of code review is to find code that will be hard to maintain

#104
post #16

What I find to be maybe the single most important part of code review is knowledge transfer. Our entire small team thumbs up a PR before it's merged unless there's a big rush on it, and this gives everyone on the team a rough idea of the state of the codebase at any given time. There's no being blindsided like "this whole system I depend on is gone" like I had happen at far more siloed places I've worked. Beyond that…

Based on your description, I don't think the process is working.

You describe a shared database, micro services you aren't aware of using the same database, and a desire for everyone to have a rough idea of the state of the codebase. And things breaking unexpectedly being caught by code review.

You have a bigger problem here of a system you can't reason about very well. The code review will help here, but I think you have bigger discoverability problems based on what I'm reading.

As others have said, what is needed is automated testing that would catch this sort of problem automatically without needing a human in the loop saying "Oh, wait...". We still want/need humans in the loop, but they should not be the only safety net.

Re: The primary purpose of code review is to find code that will be hard to maintain

#105
"it is not in general possible to find bugs by examining the code"

That really depends on the quality of developers you are dealing with.

I certainly find myself assessing code quality, performance issues, shortcomings. Occasionally trivial bugs. Most importantly you review for taste, I'm tasting the code.

Maintainability is definitely important but is pretty subjective and is a subset of taste in general.

Re: The primary purpose of code review is to find code that will be hard to maintain

#106
post #73
post #33

Earlier quoted context omitted.

User names match... are you the original author? Why commenting in the third person?

Because it was him... 10 months ago?

it's still weird. if i'm talking about something i wrote 10 months ago i'd say something like, "when i originally wrote this i was trying to say ...".

not "when the author wrote this, he was trying to say"

Re: The primary purpose of code review is to find code that will be hard to maintain

#107
post #16

What I find to be maybe the single most important part of code review is knowledge transfer. Our entire small team thumbs up a PR before it's merged unless there's a big rush on it, and this gives everyone on the team a rough idea of the state of the codebase at any given time. There's no being blindsided like "this whole system I depend on is gone" like I had happen at far more siloed places I've worked. Beyond that…

> Our entire small team thumbs up a PR before it's merged unless there's a big rush on it This has been tried by a couple of my past managers. This feels great with a small team on a slow moving codebase. If you try to force it on a larger team or expect the codebase to move quickly then it turns into a performative game of skimming the code (if that) to click the thumbs up button so you can get back to your work. Th…

This with a side helping of "when everyone is responsible nobody is"

Then again, if you're dealing with an un-maintainable minefield of a code base then "fire the whole team, I dare you" might be what you're after.

Re: The primary purpose of code review is to find code that will be hard to maintain

#108
post #14

This just makes reviewers and authors lazier. The purpose of code review is multi-faceted. Hard to maintain? Yes. Might have bugs? Yes. Can be done simpler/cleaner? Yes. Is in line with project code style? Yes. Get someone else to also understand the code? Yes. Onboard junior team member? Yes. Sanity check design decisions? Yes. This flippant note is mostly more self-justification for being a lazy code reviewer.

Totally agree. With the speed at which code can be written and deployed today due to AI, the emphasis should shift onto the review. Does the code actually run properly, are all of our assumptions correct, and are there any unintended side effects?

I've found the review and debugging process to be much more time consuming than writing/producing code, and just "praying it works" never ends well.

Re: The primary purpose of code review is to find code that will be hard to maintain

#109
Code review doesn't have a single purpose. Finding code that is hard to maintain is one of those, and and an important one, but certainly not the only one, and I'm not sure it is even the most important one. Other purposes include:

- a safety check to ensure that if a developer (or AI) goes rogue, it is more difficult to merge malicious code

- a second perspective from someone who isn't as close to the problem and might see a better way to do things, or problems that the original developer missed

- in some cases having someone more familiar with other parts of the system look at it who can tell if it won't interact well with something else

- ensuring there is at least one other person familiar with the code

- a learning opportunity. The author can learn from feedback from the review, and the reviewer can learn from the code in the change. Especially important when the author and reviwer have differing seniority. When I mentor a new employee, I add them as a reviewer to all my PRs so they can see how I do things, and review all their PRs so I can provide guidance. And sometimes I even learn things from them!

- yes, catching bugs, although this should not be the primary mechanism for that, and I agree is not the most important reason. It is especially important for security and performance bugs though, as those are harder to catch with automated testing.

Re: The primary purpose of code review is to find code that will be hard to maintain

#110
post #16

What I find to be maybe the single most important part of code review is knowledge transfer. Our entire small team thumbs up a PR before it's merged unless there's a big rush on it, and this gives everyone on the team a rough idea of the state of the codebase at any given time. There's no being blindsided like "this whole system I depend on is gone" like I had happen at far more siloed places I've worked. Beyond that…

> For instance recently I made a small change to a table and a coworker pointed out that there was a microservice I wasn't considering that wrote to that table that would break If code reviews are important, where does testing sit? Presumably if the coworker had not been part of the code review something would have stopped the breaking change making its way to prod?

I'd guess testing is done only for the software being deployed, not for that other microservice.

At least, that's what people do by default.

Post reply on HN