It's kinda funny that we all agree that code review is important but we cannot agree why exactly it is important. Imagine a bunch of ancient shamans arguing why exactly local volcano needs to be fed one virgin a year but all being in agreement that it does.
> As everyone should know by now, it is not in general possible to find bugs by examining the code.
Lost me here. I would agree that it’s not possible to find every bug by examining code, but in real code reviews bugs and errors are identified by reviewers all the time. Reviewers lend their past experience to the situation, identify some unnoticed interaction, think of an edge case that the author hadn’t considered, or some times just notice simple logic errors.
Code review is a fresh set of eyes. When we write code eventually parts of it get accepted in our minds as done or correct and we can start missing things that are obvious to a reviewer.
I’m not a fan of these blanket declarations that code review isn’t about reviewing code. I’ve read countless hot takes like this that code review is about some other thing (finding unmaintainable code, knowledge transfer, etc) that all miss the point that code review isn’t about one thing. These reductions and exclusions can be really misleading.
I would add to this other purposes of code review:
* 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
I don't know about many people, but the author for sure doesn't understand the primary purpose of code review.
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.)
Well, the code review should also be reviewing the provided test code or test plan or whatever that will prove it does not have bugs.
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.
Whatever purpose code review served pre-2002, post-2002 it serves as corporate audit coverage. A common (mis?) interpretation of SOX and SOC2 is essentially that the company must have a two-person sign-off on any system change that could damage the company or its reputation.
> 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. 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.
[flagged]
[flagged]
[dead]
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.
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.
Why are we still so obsessed about maintaining code when rewriting it is very low cost now?