Code Review Habits That Actually Improve Your Team's Code

Software EngineeringCareerDeveloper Tools

Most code reviews don't find bugs. They find variable names.

A senior engineer skims a diff between meetings, leaves a comment about naming a function handleClick instead of onClick, and approves. The PR merges. The logic error in the edge case nobody thought about ships to production three weeks later. The naming comment, obviously, did not help.

I've been on both sides of this. I've written shallow reviews when I was busy and later watched the code I approved cause an incident. I've also had my own code reviewed by someone who genuinely dug in, and the difference in what they caught versus what I expected them to catch was humbling. Good code review (the kind that actually improves code quality) is a specific skill. It's not about knowing more than the author. It's about having the right habits when you sit down to review.

This is a post about code review best practices engineers actually use to make review count. Not the theory. The habits.

Why Most Review Is Too Shallow

The problem is partly structural. Review happens at the end of the feature cycle, when the author wants to merge and the reviewer has their own work waiting. There's social pressure to approve. Nobody wants to be the person who holds up shipping.

So reviewers take the path of least resistance. Style comments are easy to make and easy to justify. Logic review requires actually understanding what the code is supposed to do, which takes effort. Naming, formatting, and structure are visible in the diff. Business logic errors, edge cases, and architecture mismatches are not visible. You have to go looking for them.

A SmartBear study of Cisco's codebase (2,500 reviews across 3.2 million lines of code) found that reviewing more than 400 lines at once causes defect detection to collapse. Reviewers stop finding defects not because the code is clean, but because their brain gives up. The same study found that inspection rates above 500 lines per hour miss a significant percentage of defects. Above 800 lines per hour, reviewers haven't really read the code at all.

I think most teams review at exactly that pace. We rush, we skim, we leave a few comments to show we were there, and we approve.

Review the Tests First

Here's a habit that changed how I approach reviews: open the test file before the implementation.

Tests tell you what the author thought the code was supposed to do. If the tests are thin, that's information. If there are no edge cases tested, that's where you should go looking. If the test names are vague (test_it_works), the author probably doesn't have a precise mental model of the behavior.

Reading tests first also gives you a checklist. You know what's been covered. Then when you read the implementation, you can ask: what isn't covered? What happens if this input is empty? What if the API call fails? What if the user is unauthenticated?

That said, this technique only works if you actually read the tests critically. Glancing at them doesn't count. You're looking for what's missing, not just confirming they exist.

What to Actually Look For

Style and naming are easy to automate. That's not where your attention should go. Here's what's worth your time:

  • Architecture fit. Does this change belong in this layer of the system? A frontend component that makes database decisions. A utility function that's growing into a service. These are worth flagging early, because they're expensive to fix later.
  • Edge cases. What happens at the boundary? Empty arrays, null values, zero amounts, expired tokens, concurrent requests. The author thought about the happy path. You think about what breaks it.
  • Error handling. Is the error thrown, swallowed, or surfaced to the user? Is the user experience reasonable when something fails? I've reviewed a lot of code where errors were caught and discarded with a console.log. That's a production incident waiting to happen.
  • Naming as communication. This is different from style pedantry. A variable called data tells you nothing. A function called processUser is a smell. Process how? Why? Names are the first documentation a future reader sees. When naming is vague, it usually means the abstraction is vague.
  • Complexity. Google's engineering practices flag over-engineering specifically: generic solutions to problems that don't exist yet, abstractions that make simple things harder to follow. Complexity is not a sign of skill. It's a liability.

How to Write Review Comments That Actually Land

Review comments fail in two ways. Either they're too vague to act on ("this is confusing"), or they're commands without reasoning ("change this to X"). Neither helps the author learn or improve the code.

The pattern I try to follow: explain the why, suggest an alternative, and acknowledge what's good.

Explaining the why turns a comment into a lesson. "I'd rename this to isExpired because boolean names that start with is or has make the call site easier to read" gives the author something to carry forward. "Rename this" does not.

Suggesting an alternative is the most useful thing you can do. If you've spotted a problem, you probably have a direction in mind. Share it. The author might know something you don't and push back, which makes the discussion better. Or they'll use your suggestion and the code will be clearer.

Acknowledging what's good isn't just nicety. It signals that you read the whole thing. "Nice use of the early return here, keeps the happy path readable" costs you five seconds and builds the kind of trust that makes critical comments easier to receive. It also means you don't stop the moment you find something wrong.

I also try to use prefixes to be clear about weight. "Nit:" tells the author this is optional. "Blocker:" tells them this must change. Ambiguous feedback creates ambiguous outcomes.

Time-Box Your Reviews

Here's a concrete habit that helps: decide how long you'll spend before you start, and stick to it.

If a PR is 300 lines, I give it 45 minutes. That's enough time to read carefully, think about edge cases, and write useful comments. If I can't review it properly in that time, the PR is probably too big to review at all.

The data backs this up. That SmartBear Cisco study found defect detection drops sharply after 60 to 90 minutes in a single session. Your brain gets fatigued. You start approving things you'd have flagged an hour earlier. Better to do a focused 45-minute review than a meandering two-hour one where you catch less.

The corollary: when a PR is too big to review well, say so. Ask the author to split it. This feels uncomfortable. Do it anyway. A 1,200-line PR has a 28% defect detection rate in review. That means 72% of the defects are getting through. That's not a review, it's a formality.

The Recurring Issues Spreadsheet

I started keeping a simple note (nothing fancy, just a list) of issues that come up repeatedly in reviews on my team. Not to shame anyone. To find patterns.

When the same thing comes up three or more times, that's a signal. Either the team doesn't know the preferred pattern, the documentation is unclear, or there's an easy automated check we haven't written. All three are solvable.

If your team keeps missing async error handling, that's a lint rule. If people consistently import from the wrong layer, that's an architectural document that doesn't exist yet or a path alias that needs adjusting. Catching individual instances in review is fine. Eliminating the class of mistake is better.

This habit also makes feedback feel less personal. "We keep catching this pattern in review, let's write a rule for it" is easier to hear than "you keep doing this."

Code Review in the Age of AI-Generated Code

This part has changed fast, and I think most teams haven't caught up.

About 41% of code written in 2025 was AI-assisted. A CodeRabbit analysis of GitHub PRs found that AI-generated code has 1.7 times more issues than human-written code: logic errors up 75%, security vulnerabilities up 1.5 to 2 times. And only 29% of developers report trusting AI-generated code, yet many teams aren't reviewing it any differently.

AI code has a specific failure mode. It looks right. The syntax is clean, the naming is fine, the structure is reasonable. But the logic can be subtly off in ways that are hard to catch in a skim. Off-by-one errors in loops, edge cases that weren't in the prompt, security patterns that are almost correct. The Stack Overflow 2025 developer survey found that 45% of developers cited "AI solutions that are almost right, but not quite" as their top frustration.

Almost right is the hardest thing to review. You have to slow down more, not less.

My practical advice: when you see a PR where large chunks were clearly AI-generated, treat it like you're auditing code you didn't write from someone you don't know. Check the edge cases more carefully. Look at the error handling with fresh suspicion. Test the tests. Make sure they cover what they claim to cover and weren't also generated to pass rather than to verify.

Also, AI-generated code often violates implicit team conventions. It doesn't know that you use a specific internal logger, that you avoid any in TypeScript even where it's technically fine, that you don't use that particular React pattern anymore because it caused perf issues two years ago. These aren't in the lint rules. They're in the team's shared knowledge. Review is how that knowledge gets transferred.

Building a Review Culture Without Being the Annoying Person

Here's the hard part. All of this is useless if reviews become adversarial.

The goal of review is to improve the code, not to demonstrate your knowledge or catch the author making mistakes. This sounds obvious but it's easy to forget when you're under time pressure or when you've found something that seems clearly wrong.

A few things that help:

  • Separate the code from the person. "This function is hard to follow" is about the code. "You wrote this in a confusing way" is about the author. The first opens a conversation. The second closes one.
  • Don't review everything every time. Pick your battles. I leave comments on things that matter: correctness, architecture, error handling, meaningful naming. I let small style things go if they're not covered by a linter. A review with 20 comments feels like an attack even if every comment is valid.
  • Don't block on preference. Google's standard is useful here: approve when the code definitively improves the overall code health, even if it isn't perfect. There is no perfect code. If you'd write it differently but both approaches are fine, say so and approve.
  • Be consistent. If you hold people to a standard in review, write code that meets that standard yourself. Nothing kills review culture faster than a reviewer who never accepts feedback on their own PRs.

I've found that the culture question is mostly about modeling. If senior engineers give thorough, kind, specific reviews, that becomes the norm. If they skim and leave sarcastic comments, that also becomes the norm. You don't fix it with a policy. You fix it by being the reviewer you'd want on your own code.

Recommended Resources