Top
Best
New

Posted by ColinWright 15 hours ago

The primary purpose of code review is to find code that will be hard to maintain(mathstodon.xyz)
324 points | 162 commentspage 7
bioninf_n_door 7 hours ago|
[flagged]
shemnon42 12 hours ago||
[flagged]
danielsmori 7 hours ago||
[flagged]
mind_bridge 9 hours ago||
[flagged]
CurbStomper 12 hours ago||
[dead]
black_13 4 hours ago||
[dead]
high_na_euv 13 hours ago||
Uh.. why not both?
pessimizer 7 hours ago||
To have two primary purposes is difficult in English.
Pxtl 13 hours ago||
Because the reviewer is not magical. If there was something in the code the author couldn't see, the reviewer probably won't see it either.

The way to confirm that code does not have bugs is testing. So the reviewer is not looking at the code saying "this will work", they're looking at the code saying "I understand how this works, it makes sense."

Evidence that the code is safe is something that also should be provided in the PR, but it is not the main code. It is ideally test automation that is just as understandable as the feature code, but failing that ad-hoc test evidence or a specific step-by-step plan with evidence of execution is good too.

cat_plus_plus 13 hours ago||
The primary purpose of code review is to maintain existing hierarchy by preventing junior SWEs from getting promoted by committing code that is smarter than what the senior architect can understand.
SketchySeaBeast 13 hours ago|
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.
sjducb 13 hours ago|||
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.

SketchySeaBeast 8 hours ago||
Great programmers write code that makes you wonder why everyone doesn't do it like that.
cat_plus_plus 13 hours ago|||
Write me simple AI inference code that has high performance in big batches.
SketchySeaBeast 13 hours ago|||
I don't know enough to speak about that particular domain, but if the junior is writing something the senior can't understand, that's always going to be a problem. That code becomes the team's responsibility, and that code needs to be able to be maintained by the entire team, not only by the junior with something to prove.

Who is getting called at 2 AM when something breaks? Not the junior.

cat_plus_plus 9 hours ago||
Hehe you think L9s do oncall? Senior not understanding is a problem all right. It's not necessarily junior's problem.
SketchySeaBeast 8 hours ago||
I feel like you keep missing the point and are more interested in gotcha's.
AlotOfReading 13 hours ago|||
Does tinygrad not count for some reason?
storus 12 hours ago|
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.
seabrookmx 12 hours ago|
If that's how it functions where you work, I'd be looking for a new job.
storus 11 hours ago||
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.
seabrookmx 7 hours ago||
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.