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

121–130 of 193 posts

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

#121

Post by hakunin »

I think the author is right.

There are many peripheral nice-to-haves you can get from a code review. Bugs, security, performance, correctness issues are all possible bonus findings you may get sometimes.

But there is only one _must_ in reviews: another person reading and understanding the code, possibly suggesting architectural improvements, or asking questions that should be answered by rephrasing the code for clarity, or by adding code comments. In other words: maintainability. That's the one thing that's not a bonus point, and is a constant for all code reviews.

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

#122

Post by levl289 »

thayne wrote: post #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 mi…

OP is Short-form false-dichotomy. Thank you for this perfect response.

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

#123

Post by dotancohen »

a4isms wrote: post #115
thayne wrote: post #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 mi…

Another—very old—rationale: People write code differently when they know that it will be reviewed by people who will not only comment on it, but also form long-term impressions of the submitter's competence and fit based on the code that is reviewed.

I've always felt that this is and advantage to open source software. The vast majority of open source software that I've bothered to look at the code for used best practices, was reasonably secure, and was above all maintainable. The bespoke projects that I've worked on at various companies? Complete spaghetti messes almost all of them.

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

#124

Post by ivanjermakov »

thayne wrote: post #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 mi…

Similar to #3:

- having someone more experienced in that subdomain catch problematic parts and inconsistencies with existing code.

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

#125

Post by dijksterhuis »

havblue wrote: post #97

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…

without knowing the people involved, or any further nuance/context -- i had a PhD supervisor like this. he was always looking for holes. always disliked the way i was proposing something. always looking to find the thing that's wrong. the way to deal with him was to have thorough answers to as many questions he might ask before he asks them. learning to predict his questions was the real training for my PhD.

it taught me to show my working, as if i were in math class. which meant fully writing things up before meeting with him.

> how the reviewer would prefer to design it

sounds like they're trying to find holes. dunno if you do this already, but if not, try to fill the holes beforehand with conclusive and extensive write up (show the working). maybe ask them before coding how they would design X and then dedicate a section of the write up to why X cannot or was not designed that way.

answer their questions before they have a chance to ask the questions.

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

#126

Post by threethirtytwo »

thayne wrote: post #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 mi…

I agree it doesn’t have a single purpose but you didn’t refute the claim about a primary purpose. One purpose to rule them all.

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

#127

Post by boie0025 »

thayne wrote: post #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 mi…

I agree with all of this, I read into the "single purpose" of understanding the code and complaining about what you don't understand implying that if you understand it, you will be able to point out and comment on things that are wrong /foolish/unsafe/etc after understanding it. From that perspective on the OP, it makes sense to me. Particularly with regard to modularity and factoring; once I understand all of a gigantic PR I will have modeled it in my mind and will begin to either see that it will be maintainable, or will be a total nightmare one day... or somewhere in between.

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

#128

Post by k3vinw »

I appreciate the perspective, but code reviews are subjective. The tooling and the language can be so good that it shifts to this kind of utopian state, where all bugs are caught and eliminated by guarantees in the language and tooling. Or they could be dismal to the point a human needs to check for mundane issues like bugs in the code.

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

#129

Post by peder »

misja111 wrote: post #57

Nowadays the main point of PR's is to find out what Claude has been coding for me.

lol, but also, nearly every comment in this thread seems so old school, as if the game hasn't completely changed.

PRs for knowledge transfer? I don't need it, I'll just ask an LLM a question if I have one.

PRs for enforcing good design? I dunno, if it's a terrible design, we'll just change it since change is so much less work now.

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

#130

Post by Splice9160 »

Maintainability is incredibly important, but acting like bug-catching is just a "happy little accident" of code review is an over correction. A good code review should make sure the code is readable and that it actually does what it’s supposed to do without breaking the system. You need to do both.
Post reply on HN