Code Reviews That Actually Help: What I Learned After a Year of Giving Bad Ones
"I was reviewing the code to find problems. I should have been reviewing the code to understand it, and treating the problems I found as a consequence of that understanding, not the goal."
For the first year I gave code reviews, I thought I was doing them correctly. I read the diff, I identified issues, I left comments, I approved or requested changes. The process felt professional. I was finding real bugs. I was enforcing style consistency. I was pushing back on approaches I thought were wrong.
Then I got a piece of feedback from a junior engineer on the team that made me rethink the whole thing. She told me, in a one-on-one, that she had started writing defensive comments in her PRs before submitting them, pre-explaining choices she thought I would object to, to forestall comments that felt more like challenges than questions. She said she dreaded my reviews.
I hadn't intended any of this. But intent isn't the relevant measure when someone is writing defensive preambles before asking for review. The outcome was that a junior engineer was spending energy managing my reactions rather than writing good code, and she was doing it because my review process had made her feel like the goal was to defend her work rather than improve it.
What I Was Actually Doing
Looking back at my reviews, the pattern was clear. I was reviewing the code to find problems. Every comment was a problem: a bug, an inefficiency, a style violation, a design choice I would have made differently. The ratio of problems found to questions asked was probably 10:1. I wasn't exploring the code with curiosity, I was auditing it with suspicion.
This created a review dynamic where the author's job was to defend every decision I questioned. The author was correct to approach it that way, because my comments were implicitly framed as criticisms rather than questions. "This will cause a problem when X" reads differently from "Have you thought about what happens when X?" The first is a verdict. The second is an invitation to think together.
The other problem: I was giving the same depth of feedback on everything. A naming inconsistency and a potential race condition got the same comment format, the same tone, the same implied weight. The author couldn't tell what was important and what was a preference. When everything is treated as equally significant, nothing is.
What a Useful Review Actually Does
A useful code review has a few distinct things it's trying to accomplish:
It tries to understand what the code is doing before it evaluates how well it's doing it. The first pass through a PR should be reading comprehension, following the logic, understanding the flow, building a mental model of what the change accomplishes. Comments that come from genuine confusion or curiosity ("I'm not sure I follow this step, is the intent X?") are almost always more useful than comments that come from immediate judgment.
It distinguishes between correctness issues and preference issues, and makes that distinction explicit. A comment that identifies a genuine bug or a performance problem that will affect users in production is categorically different from a comment that suggests a different variable naming convention or an alternative implementation approach that would also work. Conflating them wastes everyone's time and obscures what actually matters. I started prefixing preference comments with "nit:" and structuring the review summary around the required changes first, optional suggestions second, and questions third.
It teaches, not just corrects. When I find something that needs to change, the most useful comment explains why, not just "use a map here" but "a map here gives O(1) lookup instead of O(n) which matters because this function runs on every request." The author learns something that applies beyond this PR. The comment also invites disagreement: maybe the function doesn't run on every request, and I was wrong about the context. The explanation opens a conversation. The directive closes it.
The Hardest Part: Letting Things Through
The most important shift in how I give reviews was learning to let through code that I would have written differently but that works correctly and is maintainable. My way is not the only way. An implementation that solves the problem cleanly, even if it's not the approach I would have chosen, doesn't need a comment suggesting the author should have chosen my way instead.
This is harder than it sounds when you have strong opinions about code quality. I genuinely believe that certain patterns are better than others and that it matters which ones a team uses. But "better" in most cases means "better in the specific context of this codebase, given the team's conventions, given the constraints of this feature." If I'm not thinking about those specifics when I suggest a change, I'm often substituting my general preference for actual analysis of what's right for this case.
The test I now apply: would this change make the code harder to break, harder to misread, or faster to run in a way that matters? If the honest answer is no, and the existing code is correct and clear, it probably doesn't need a change, it needs me to let it through.
Reviewing Junior Engineers Versus Peers
These are not the same activity, and treating them the same is a mistake I made early and often.
When reviewing a junior engineer's code, the most valuable thing a reviewer can do is explain the reasoning behind feedback in enough detail that the author can apply the same reasoning to the next PR without needing the same feedback again. A comment that says "this needs a null check" doesn't accomplish this. A comment that says "this will throw if getUser() returns null, which it can when the session has expired, you can see an example of how we handle this in UserService.getOrCreate()" gives the author a pattern to carry forward.
When reviewing a peer's code, especially someone more senior, the dynamic is different. I've gotten into the habit of asking more questions and stating fewer conclusions. "I'm not sure I understand the choice to use a queue here rather than a direct call, what's the advantage in this case?" is not a challenge to a senior engineer's judgment; it's a request to learn something. Often the answer is illuminating. Sometimes the answer is "actually, you're right, let me rethink that."
What Changed After I Changed
The junior engineer who told me she dreaded my reviews became one of the more confident contributors on the team over the next year. She stopped writing defensive PR preambles. She pushed back on my comments when she thought I was wrong, and was right more often than I expected. Her code got better, but more importantly, her relationship with her own code changed. She wrote as someone who expected her code to be understood and engaged with, not as someone who expected to defend it.
I can't attribute all of that to my change in review approach. But I know the previous dynamic wasn't helping, and I know the new one was at least not in the way. The goal of a code review is a better codebase and a more capable team. Both of those outcomes require the people involved to feel like the process is collaborative rather than adversarial. That feeling comes from the reviewer's intent, the reviewer's approach, and the reviewer's willingness to treat the author as a partner rather than a subject of evaluation.
Getting that right is harder than finding bugs. It took me longer to learn. I think it matters more.
Tags