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

91–100 of 193 posts

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

#91

Post by razemio »

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.

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

#92

Post by kibwen »

Sharlin wrote: post #83
dude250711 wrote: post #43

Earlier quoted context omitted.

Such a luxury, I am envious! Our team started using AI, so I switched to a simple method: no comments, and a binary "is this batshit crazy or passable" approval decision rule. Saving myself time and sanity.

In other words, AI code is owned by nobody.

It's not innacurate to say that AI code becomes legacy code the instant it's merged.

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

#93

Post by giancarlostoro »

The other issue with code review, and I'm glad I've not worked with people like this anymore, for the person being reviewed: NOBODY IS ATTACKING YOU, nobody is saying your code is bad, the goal is to do a once-over for quality.

Another goal people often miss:

It's okay to ask "stupid questions" and I would argue as a Junior, ask away, even if no code changes happen, ASK. Kind of follows the spirit of the original post, which is, can you maintain this?

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

#94

Post by whilenot-dev »

> 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 should ever be steps that merely want to be satified - work with these steps and treat them as the valuable tools they are, and never work against them.

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

#95

Post by oh_my_goodness »

giancarlostoro wrote: post #93

The other issue with code review, and I'm glad I've not worked with people like this anymore, for the person being reviewed: NOBODY IS ATTACKING YOU, nobody is saying your code is bad, the goal is to do a once-over for quality. Another goal people often miss: It's okay to ask "stupid questions" and I would argue as a Junior, ask away, even if no code changes happen, ASK. Kind of follows the spirit of the original pos…

>NOBODY IS ATTACKING YOU

Somehow this captures a lot of the culture for me.

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

#96

Post by Aurornis »

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…

> 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.

The end game was a situation where nobody was really reviewing code because everyone had their own work to do and they didn’t want to be the one person blocking important PRs, so everyone was clicking thumbs up.

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

#97

Post by havblue »

I've been at a lot of companies where code is held hostage by the reviewers, specifically leads or wannabe leads. Interfaces matter. Maintainability matters. Memory allocation certainly matters. Code reviews frequently go down the rabbit hole of how the reviewer would prefer to design it and a long back and forth until the developer has, "explained himself", like it's a Breaking Bad confrontation. I think it devolves into a status battle.

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

#100

Post by Aurornis »

> As everyone should know by now, it is not in general possible to find bugs by examining the code.

Lost me here. I would agree that it’s not possible to find every bug by examining code, but in real code reviews bugs and errors are identified by reviewers all the time. Reviewers lend their past experience to the situation, identify some unnoticed interaction, think of an edge case that the author hadn’t considered, or some times just notice simple logic errors.

Code review is a fresh set of eyes. When we write code eventually parts of it get accepted in our minds as done or correct and we can start missing things that are obvious to a reviewer.

I’m not a fan of these blanket declarations that code review isn’t about reviewing code. I’ve read countless hot takes like this that code review is about some other thing (finding unmaintainable code, knowledge transfer, etc) that all miss the point that code review isn’t about one thing. These reductions and exclusions can be really misleading.

Post reply on HN