People will generally copy and follow existing patterns, so for example if you let somebody add a new internal date time format, then soon your codebase will bifurcate and there'll be multiple inconsistent versions roaming around.

The other stuff (minor bugs, overly verbose code) can easily be fixed. Paradigm rot cannot.

Maintainability is a major factor in that, of course.

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. Everything on top of that is extra. I've found real and significant bugs doing code review. In large part because I understand the codebase better than the author. That's finding and preventing bugs.

I also read PRs looking for malicious code trying to sneak in, you never know, the person I've called my best friend, someone with opsec better than mine, may suddenly have turned evil and this is the PR where they're finally sneaking in the back door.... that's never happened yet, but fingers crossed!

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

... I'd love to know what the author really meant to write here, because it certainly wasn't this.

The purpose of code review is multi-faceted. Hard to maintain? Yes. Might have bugs? Yes. Can be done simpler/cleaner? Yes. Is in line with project code style? Yes. Get someone else to also understand the code? Yes. Onboard junior team member? Yes. Sanity check design decisions? Yes.

This flippant note is mostly more self-justification for being a lazy code reviewer.

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 follows "The point is..." varies between so many different people. I can't quite say it's a unique take per person, as I've seen before, there are some common threads, but they are also not all the same answer by any means either.

In this case, there isn't a "the" point of code review to discuss. It turns out that while we all may have thought we were doing it for the same reasons, we aren't. This is real. We don't have the same goals, we don't have the same methodology, and thus, the value we get from it may be different. And in fact it is perfectly reasonable to discuss the multiple cost/benefits ratios that differ across the various definitions, because the simplification "it's good, end of story" is destroying important distinctions.

In this situation, it is helpful to frame this as a matter of the costs and benefits of the various options available. Forget the statement "code review is good"; it is fallacious to start with that statement as an axiom and then argue about whether or not your definition of "code review" is or is not the "correct" definition so that your definition gets the "good" attribute applied to it. Consider the options directly.

(I have to admit I've used this effect in anger... in meetings where I can tell that everybody thinks they know what some project is but I can tell they all have a different definition of it in mind, but I also know it's not going to happen anyhow, I don't chase down the differences. Sometimes you can use this to your advantage to cut short what would otherwise be a quite interminable, yet ultimately pointless, meeting.)

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, it gives a forum to ask questions about how things work to further build understanding. On a high functioning team, every developer should have at least a modest understanding of the entire system, including parts they never touch.

Another important feature is just the institutional knowledge check. For instance recently I made a small change to a table and a coworker pointed out that there was a microservice I wasn't considering that wrote to that table that would break (yes, sharing tables is bad design, unrelated). I had no idea this microservice existed let alone had access to this table. The institutional knowledge check here though prevented a larger issue and potential data cleanup situation.

This makes me wonder if we all have a different primary purpose in mind when it comes to code reviews because that wouldn't be my number one. Talk within your teams would be my advice. Especially now with AI enabling more rapid changes.

GitHub style asynchronous pull request review with inline comments is the norm now, but it’s not the only sort of review there is. I’m old enough to remember processes that include in person reviews that were more like a dissertation defense or conference presentation.

The literature around this that shows that code review is a useful quality practice (in fact one of the only useful quality practices) comes mostly from much more structured review processes than we see now.

My personal opinion is that before llms the GitHub style pr review was for making us feel better about our processes (or governance checkbox checking) and the age of llms will sweep them away as the cost/benefit is so much worse now.

This is why the solutions for high-trust environments (small teams) and low-trust environments (big companies, open source projects) will be different.

https://crawshaw.io/blog/agent-principal-agent

You're not reviewing the code to confirm that the code is bug free... you're reviewing the additional code that confirms that the feature-code is bug free.

Any process that has a step of "we'll get to that later" is a failure. That includes testing. Until there is some provided content that will be able to provide evidence that that code is safe to merge, it's not done.

But yeah, I need to be able to understand what every line does.

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 PRs. Why spend the time doing a thorough review if it's going to get rejected anyways.

Well kinda - code review needs to identify any missing tests? And without the tests more likely a bug could exist.

If the primary purpose of code review is to assess maintainability, there is no need for review, that can be done by automated tooling (formatting, bad naming, cyclomatic complexity etc.)

In order words, the purpose of code review is to or not ask for changes on the code.

And I agree to some extent with what the OP's tweeter said. these days, bugs can look perfectly fine on the surface, but when combined with the existing system, entirely new types of bugs emerge. This is an especially common pattern in the AI era: the added code itself isn't the problem, but it becomes a bug once it interacts with the existing codebase.

> “Programs must be written for people to read, and only incidentally for machines to execute.”

people probably did not realise what it meant, but AI is bringing it to forefront. Huge amount of work we do is essentially to communicate decisions we want to take, are going to take or have taken. It is a cornerstone of our society when people are continuously exposed to the relevant decisions which are taken in order to build a shared understanding and move forward.

Programming was nothing but that. We did not have a good enough compiler till now and we definitely do not have a good enough language to describe what we need (mostly because thoughts and world in general are so much more complex than any language). And therefore, we used the same language for computer to execute our code and for us to read and understand it. But the reason we store our code in human readable language and not machine code is because we want to communicate the code to future self.

That's why Elon musks's statement about just directly getting a binary makes no sense, because the language used to specify that binary needs to be stored in some engineering records anyway, and then "that is the code".

Code review is also exactly the same, it is a signal which says, "I want to take this decision, are you okay with us taking this decision?" and everyone interested signs off.

Bugs finding are just people going, "I completely agree with the principle of the decision but this particular part of decision is anti-thetic to the principle, should we fix it?"

* to share knowledge among the team — if code has been reviewed then at least two people know about it

* to ensure the changes won’t cause problems with other systems or future plans — maybe we will be rolling out a new logging system soon and we don’t want to solve the problem in this specific way

* a chance for the reviewer and reviewee to learn something about the system or the code via the review discussion

* team coherence — code that’s well reviewed is a team effort. Working together on small things like code reviews helps teammates work better together on other tasks in the future