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

161–170 of 193 posts

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

#161

Post by jibal »

smcameron wrote: post #39
grayhatter wrote: post #12

> many people misunderstand the purpose of code review Oooh, I bet including the author? Yeah, right there, he fails to make any qualifications for his statement, making it factually incorrect. There are plenty of reasons to do code review. If you force me to, I'll define it as information transfer. The point is to have a conversation about the code. To expand both people's understanding about the codebase. Everythin…

He's a mathematician, so what he means by "in general", is "in every possible case", or "without exception", so what I think he means is, "not all bugs will be found by code review." I agree it probably could have been made more clear.

But a) that's not what his words mean in English and b) that's irrelevant.

"in general" quantifies over all occasions, not over all bugs on one occasion.

To quote my own response:

===

Apparently the mathematician author doesn't understand the meaning of his own natural language quantifiers. “it is not in general possible to find bugs by examining the code” means “it is not in general possible to find ANY bugs by examining the code”, not “it is not in general possible to find ALL bugs by examining the code”.

And the first interpretation is relevant but wrong, whereas the second interpretation is true but irrelevant.

P.S. It seems that the author meant to say “it is not in general possible to find a given bug by examining the code”, i.e., "not (for all bugs B it is possible to find B)", which again is true but not relevant.

===

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

#162

Post by seabrookmx »

storus wrote: post #102
seabrookmx wrote: post #75

Earlier quoted context omitted.

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.

Sure, but that doesn't mean it works that way everywhere or is inevitable.

Where I work, a senior would be reprimanded for such behavior. A key metric to their success is how well they can transfer knowledge and teach others, and their peers' feedback is used to determine this.

The senior that "punches down" does not go far in this case.

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

#163

Post by eptcyka »

ivanjermakov wrote: post #124
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.

And having someone less experienced can also bring perspectives and criticism to assumptions that can and sometimes should be challenged.

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

#164

Post by esafak »

peder wrote: post #129
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.

Do you even review the code then?

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

#165

Post by xlii »

I formed (and am advocating) a theory that code reviews are three dimensional space exercising trust, need for control and awareness propagation.

E.g. LGTM could be categorized as high trust, low control, high awareness.

I'm still in process of forming it on writing mostly because I get hooked up with pathological scenarios and these are infinite source of jokes to me. Some I written down already:

https://xlii.space/fiction/approved_unread_shipped/

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

#166

Post by stronglikedan »

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…

> "this whole system I depend on is gone"

This would be caught upstream in my org. If something was removed in a PR which was not documented and discussed with the entire team ahead of time, that would not be good for the submitter.

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

#167

Post by stronglikedan »

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.

AI is just another tool in a developer's toolbox. The developer owns their output, regardless of the tools they used. Same as it always has been.

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

#168

Post by win311fwg »

jerf wrote: post #15

One of my favorite little things to notice is when everybody thinks they know what something is, and they all agree about it, but they in fact don't agree. In this case we have the statement "Code review is a good idea". What right-minded software engineer could possibly disagree with that? But then notice 1. the number of people jumping up to say "No, you don't understand the point of code review" and 2. how what fo…

> What right-minded software engineer could possibly disagree with that?

No matter what "code review" ultimately is intended to mean it will have tradeoffs making its use a bad idea at least sometimes. There is no engineer who could agree with that statement. Suffice to say that the software industry attracts a lot of non-engineers, however.

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

#169

Post by janalsncm »

The purpose of code review is to validate and socialize a change. Machines can get better and better at finding bugs, but until they are hooked up directly to our brains they cannot socialize changes because that involves human understanding.

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

A code review doesn’t need to find every possible bug to be useful. Unit tests also don’t catch all bugs. They’re still a good idea.

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

#170

Post by janalsncm »

saghm wrote: post #145
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.

I read it as a reference to some theoretical limitation like the Halting Problem.

Fine, in general it is not possible to tell if code will halt. But if I see a while(1) I’m going to check if the loop can break.

Post reply on HN