Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

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

mathstodon.xyz

Post reply on HN
Print view View original

141–150 of 193 posts

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

#141

Post by skydhash »

6LLvveMx2koXfwn wrote: post #54
donatj wrote: 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?

> If code reviews are important, where does testing sit?

Testing is for general boundaries. If you have a formalized specs, investing in an harness and writing a lot of tests is worth it.

But more likely in a corporate system, fully documenting everything is a huge hurdle by itself. So your best bet is to gather everyone that is related to a change and let them evaluate its impact.

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

#142

Post by pessimizer »

razemio wrote: post #91
faitswulff wrote: post #76

Code review isn’t a singular thing. There are many reasons for code review, like knowledge sharing, liability laundering, code quality, regulatory compliance, etc. As usual, what purpose it serves depend on your use case.

Thank you! I find it somewhat ignorant to say, most people do not understand the reason behind a peer review, while obviously being missinformed. To even believe that it only serves a single purpose somewhat tells me, that the author is missing experiences with other teams / people.

> To even believe that it only serves a single purpose

There are a lot of people who incepted the word "single" into this post in order to have something worthless to argue about. Even with "primary" as the second word of the headline.

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

#143

Post by pessimizer »

silva97 wrote: post #52

The author seens to misunderstand the purpose of code review. The purpose is, literally, review the code. Review means basically to think/talk about something again in order to make or not changes on it. When you review something (including code), you are basically asking for yourself: "Should it be changed or it's okay to stay like this?" In order words, the purpose of code review is to or not ask for changes on the…

> Review means basically to think/talk about something again

The rest is not part of the definition; critics don't review movies in order to change them.

But just accepting that at face value, making code more readable is a "change." I don't know what you're trying to say here.

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

#144

Post by SketchySeaBeast »

sjducb wrote: post #28
SketchySeaBeast wrote: post #7

Earlier quoted context omitted.

If the code is so smart that it's not easily understandable, it's not easily fixable. My transition from junior to senior was accompanied by the realization that simpler is nearly always better.

Good programmers write code that’s so simple and obvious it looks like anyone could have written it. Bad programmers make the simplest things really complicated.

Great programmers write code that makes you wonder why everyone doesn't do it like that.

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

#145

Post by saghm »

whilenot-dev wrote: post #94

> it is not in general possible to find bugs by examining the code. Oh hell yes it is, at every level of abstraction even. We call those things code smell... A file descriptor that hasn't been closed, a coroutine that hasn't been awaited, a big try/catch block that just falls back to some value without logging the error, wrong type castings, etc. As a general rule: Neither type checker, nor compiler, nor runtime shou…

Yeah, I have no idea what they're talking about with that one. I've caught bugs when reviewing code without needing to run it before, and I've had the same happen to me in reverse, and I've seen it happen between others on reviews I was observing. I guess they could find some way to define "in general" so that this is technically true, but at that point it's not particularly meaningful.

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

#147

Post by saghm »

mjd wrote: post #13

The author is a mathematician, so when he says “it is not in general possible to find bugs by examining the code” he does not mean it is completely impossible to find bugs. He means only that it is not possible to find all bugs or even any particular bug.

I guess that makes sense. Based on my math lectures during college, mathematicians can often be terrible at communication to other humans, so that would explain why they think what they said is different from how pretty much everyone else reads it.

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

#148

Post by saghm »

dolmen wrote: post #73
d-us-vb wrote: 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?

saghm still thought this was a weird explanation when he wrote this comment

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

#149

Post by tarentel »

morganherlocker wrote: post #135
acedTrex wrote: post #64

Earlier quoted context omitted.

Yep, without a decent team culture this is what LLMs force, the slop deluge is just overwhelming without leadership asserting "no, stop" Ultimately you just let bugs through because the alternative is spend an inordinate amount of time communicating with someones claude through PR comments about what the shape should be. Career was fun while it lasted. I suppose its a blessing a to get to do a job that you enjoyed fo…

Disapprove and ask for a call where the author must verbally explain the changes to receive approval? This seems like a solvable problem, and one that already existed in repos with many contributors of varying skill (open source, bigco with lots of interns). Letting bugs through is an even bigger time sink.

If you work at a small shop that may work. Everyone is 100xer now thanks to AI. No one has time to actually go through all this nonsense, not even the people who "wrote" it. Approve and pray you're not the one on call is the only viable strategy for some.
Post reply on HN