In most projects I worked on it is plain burocracy to ensure the solution architect is happy with their hexagonal or clean architecture.
Really I can count up to five the amount of projects where good code review actually took place.
The primary purpose of code review is to find code that will be hard to maintain
81–90 of 193 posts
Re: The primary purpose of code review is to find code that will be hard to maintain
#82There more things in a pull request other that just reviews. It can run a build pipeline and give feedback, provision test instances to run quick tests.
It can also be used to add context and have a conversation around the why's before merging. And yeah while it's not the main purpose we've caught a lot of bugs too just from the statically reviewing code.
Re: The primary purpose of code review is to find code that will be hard to maintain
#83My attitude has always been that code review is best thought of as the gate where code goes from being owned by the author to being owned by the team or project. The code I'm reviewing is not your code, it is code that is about to become our code. Maintainability is a major factor in that, of course.
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.
Re: The primary purpose of code review is to find code that will be hard to maintain
#84> If I work until I'm exhausted and find three bugs, someone might still complain later that I missed a fourth and I should have tried harder.
This is a weird take. Less bugs is less bugs, just because you maybe didn't find them all doesn't undermine the value of finding some.
Re: The primary purpose of code review is to find code that will be hard to maintain
#85I work with someone who tends to rejects PR suggestions. I also work with someone else who accepts suggestions. I think that the for the person who accepts suggestions, it's made me wonder if they accept them in part to share ownership with me. I feel like we both maintain and understand the code, and are on the same page. For the person who rejects PR suggestions, it makes me less inclined to participate in those PR…
Our team tends to prefix all our comments with one of
* thought: Maybe foo'ing is more common in the future - we can refactor if that happens.
* change: This is a leaky abstraction, would prefer to see this modeled like bar instead.
* nit: Naming seems a little unintuitive, consider "Baz", "Boo" maybe?
* fix: This unit test is validating the wrong field.
* chat: This is a big decision and would dictate how solutions of this category look like going forward. Let's bring this to the team first.
----
With the idea that some of those prefixes are stopping the PR until they are changed, and some are just a "take it or leave it" type comments. It makes it unambiguous to the opener that you consider these X things as "We've gotta get on the same page" and these Y things as "Stated preferences" or "just an observation".
word of warning - don't feel bad if you leave a nit, the other person disagrees and ignores it. If you felt strongly about it, it shouldn't have been a nit.
Re: The primary purpose of code review is to find code that will be hard to maintain
#86[dead]
Re: The primary purpose of code review is to find code that will be hard to maintain
#87What 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, and this gives everyone on the team a rough idea of the state of the codebase at any given time.
For non trivial or chore updates a second pair of eyes is always a good idea. But it’s not possible to scale out “everybody reads everything” to a large N. The problem is that nobody could keep up with that ad the reader when there are some huge number of things to read. That’s why we delegate, create docs, and have overview sessions.
Re: The primary purpose of code review is to find code that will be hard to maintain
#88Earlier quoted context omitted.
this seems like a chain of good practices. though I find it hard to stay disciplined about keeping commits well scoped and well described
Becoming very comfortable with "rebase --interactive" and other cmds for editing your (local!) history before merging helps a lot. Once you are, it only adds 5m or so of extra work to most PRs. And while acquiring this knowledge used to be difficult, LLMs make it very easy these days.
I would also recommend an editor designed for rebases. I use the nodejs rebase-editor TUI (though it looks like the old non-vibecoded releases have been removed from github, so unclear on the current availability of this one) which makes it easier to organize
Re: The primary purpose of code review is to find code that will be hard to maintain
#89Earlier 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 code is owned by owners and society is figuring out in real-time if code has some inherent value without humans that understand it.
I'll be really interested to follow what comes out of the Bun team.
Re: The primary purpose of code review is to find code that will be hard to maintain
#90The purpose of code review is to talk about the purpose of code review ...
I don't know, but this new definition seems to be very AI friendly and matches the recent transformation of this blogger.