Skip to content
All articles
Code ReviewEngineering Culture

The Art of Code Review: What I Look For

My PR review philosophy after reviewing thousands of pull requests across three companies.

12 March 20267 min read

Code Review Is Not Bug Hunting

The most common misconception about code review is that its primary purpose is to catch bugs. Automated tests catch bugs. Linters catch style issues. Code review exists to ensure maintainability, share knowledge, and enforce architectural decisions.

When I review a PR, I'm asking myself: will a new engineer understand this code six months from now? Does this change fit the patterns established in the codebase? Are there edge cases the author might not have considered? These questions matter more than whether the code 'works.'

The First Pass: Intent and Architecture

I always start with the PR description. A good description tells me what problem is being solved, why this approach was chosen, and what alternatives were considered. If the PR description is empty or says 'fixes bug,' I ask the author to add context before I review the code.

On the first pass I look at the file-level diff to understand the shape of the change. How many files are touched? Are changes concentrated in one area or scattered across the codebase? A change that touches 15 files across 5 directories is a red flag — it either needs to be split into smaller PRs or there's an abstraction missing.

I also check if the change is consistent with existing patterns. If the codebase uses custom hooks for data fetching, and this PR introduces a useEffect with a manual fetch call, that's a conversation we need to have regardless of whether the code works.

The Second Pass: Implementation Details

On the second pass I read the code line by line. I'm looking for unnecessary complexity, missing error handling, potential performance issues, and naming that doesn't communicate intent.

The most valuable review comments I leave are questions, not directives. Instead of saying 'rename this variable,' I ask 'what does processedData contain? A more specific name would help readers understand this function.' Questions invite discussion. Directives invite defensiveness.

I pay special attention to data flow. Where does the data come from? How is it transformed? Where does it end up? If I can't trace the data flow by reading the code linearly, the code needs restructuring.

What I Never Comment On

I don't comment on formatting, import order, or any issue that a linter or formatter should catch. If your team is arguing about semicolons in PR reviews, you need Prettier, not better reviewers.

I also avoid nitpicking on purely stylistic preferences when the existing code has no established convention. If both approaches are equally readable and maintainable, I approve and move on. Your opinion about ternaries vs if-else is not worth blocking a deploy.

The goal is to be the reviewer you wish you had. Thorough but not pedantic. Opinionated but not dogmatic. Focused on the codebase's long-term health, not on proving how smart you are.

Reviewing Your Own Code

Before I submit any PR, I review my own diff in the GitHub UI as if I were reviewing someone else's code. I catch at least one issue every single time — a leftover console.log, a TODO I forgot to address, or a variable name that made sense at 2 AM but doesn't in the morning.

Self-review is the single highest-leverage habit I've developed as an engineer. It reduces review cycles, catches embarrassing mistakes, and demonstrates professionalism. It takes five minutes and saves hours of back-and-forth.

Found this useful? I write about engineering, performance, and career growth.