Code Reviews That Don't Suck: A Field Guide

So you're asked to review a peer's pull request. It's huge. 1,200 lines of changes, touching three services. Your first instinct might be to skim, leave a couple of comments, and approve just to move on. I've been there. It's a trap.

First: Push Back on Size

Google's own engineering practices say PRs should stay under 400-500 lines. And believe me, they're not being precious. I once reviewed a 2,000-line monster that took me three days to get through, and I still missed a critical bug. So when you see something that big, your first move is to ask the author to split it. Not because you're difficult, but because you want to actually catch the damn bugs. It's not about being nice—it's about effectiveness.

Let the Machines Do the Boring Stuff

Before you read a single line of logic, make sure the linters, format checkers, and static analysis tools are running. If your repo doesn't have them, that's a separate problem. But for now, assume they're working. That frees you to think about the stuff machines are terrible at: does the architecture hold together? Are the tests testing the right things? Will this be a nightmare to maintain in six months? I've seen too many reviews get bogged down in style nitpicks when the real issue is the whole approach is flawed.

What to Actually Look For

You can't check everything in every review. So prioritize. Start with functionality: does it do what the ticket says? Then testing: are there tests for the happy path and the edge cases? And if the code touches user input, security is non-negotiable. A quick scan for SQL injection, hardcoded credentials, or path traversal can save you from a post-launch disaster. I remember one PR where a developer hardcoded an API key in the frontend code. That's a blocking issue. No debate.

The Golden Rule: Critique Code, Not People

Here's where most reviews go sideways. You see a messy function and you want to write, "This is sloppy." Stop. I've been on both sides of that comment, and it never ends well. Instead, say, "This function has a bug when the input is empty—can you add a guard?" The difference is subtle but massive. One is an attack, the other is a request for a fix. And always offer a solution or at least point to a specific line. Nobody knows what "this could be better" means.

Label Your Comments

Not all comments are equal. Use labels like "nit" for style, "suggestion" for improvements, and "blocking" for must-fix issues. This helps the author prioritize. A blocking comment should be rare—reserved for actual bugs or security holes. A nit is just a preference. Don't block on it. I once saw a reviewer block a merge because they preferred single quotes over double. That's a jerk move.

When Comments Aren't Enough

Sometimes a PR is so tangled that inline comments aren't enough. That's when you escalate to a structured walkthrough, or even a Fagan inspection—the most formal kind of review, with defined stages and a moderator. You probably won't use that for a daily PR, but knowing it exists helps you recognize when a simple review is inadequate. For most cases, a well-organized set of comments and a summary is fine. But if the code is mission-critical or the architecture is shaky, call for a session.

One More Thing: Don't Skip the Human

Remember, you're not a gatekeeper; you're a coach. Keep PRs small, let automation handle the trivia, and focus your brain on logic and design. When you comment, be specific and kind. And if you see a 1,200-line PR, send it back. Your future self will thank you.

Sources

• Google Engineering Practices (code review) - https://google.github.io/eng-practices/review/
• SonarQube (GEANT KB) - https://kb.pert.geant.net/pages/viewpage.action?pageId=412221495
• Code review terminology - https://en.wikipedia.org/wiki/Code_review