Most developers learn to write code before they learn to review it. Code review is not just reading — it is reading with the goal of evaluating, communicating, and improving someone else's work. That combination of skills is rarely taught explicitly, which is why so many reviews miss the important things and focus on the trivial ones.
When you read code for yourself, you only need to understand it. When you review code, you need to understand it, evaluate it, and then communicate your evaluation in a way that helps the author — not just a way that demonstrates you found something.
This distinction matters because the communication layer changes what you look for. A flaw you cannot explain clearly is not ready to be flagged. A concern you cannot articulate as a concrete consequence — "this will cause X under condition Y" — is not ready to be written as a review comment. The review is not complete when you finish reading; it is complete when your comments would help a thoughtful author improve the code.
A common failure mode in code review is spending thirty minutes on variable naming and missing a logic bug. Prioritize in layers: correctness first, then design, then style.
Correctness: does the code do what it claims? Are there edge cases that break it — empty inputs, null values, concurrent access, integer overflow? Does it handle errors or silently swallow them? These are the bugs that reach production.
Design: is the abstraction right? Does this function do one thing? Is this state managed in the right place? Will this design make the next change harder? Design issues are not bugs today — they are bugs in three months when someone adds a feature.
Style: naming, formatting, comment quality. These matter for maintainability, but they should never crowd out correctness and design. If your review is mostly style comments, you reviewed the wrong layer.
A pull request diff shows you what changed, not what is. Reading only the diff is fast but dangerous — it gives you no context about how the changed code fits the surrounding system. Reading the full file is slow but necessary for design-level review.
A practical approach: read the diff first to understand the author's intent, then read the surrounding context for the functions that changed. For each changed function, ask: does the change make sense given the full contract of this function? Does it preserve the invariants the callers expect?
For large PRs, resist the urge to approve based on the diff alone. Large diffs that look locally correct often contain integration bugs that only appear when you read the callsites.
The most common bad review comment is a statement without reasoning: "this is wrong" or "use a map here." The author does not know why it is wrong or what problem a map solves. The comment creates friction without creating understanding.
A useful comment has three parts: what you observed, why it matters, and a concrete suggestion or question. "This loop runs in O(n²) because it calls findById inside the iteration — if the list grows past a few hundred items this will be slow. Could we build a lookup map before the loop?" That comment teaches, not just corrects.
Distinguish between blocking issues and non-blocking suggestions. Blocking: the code should not merge as written. Non-blocking: a suggestion worth considering, but not a requirement. Making this distinction explicit saves both reviewer and author from ambiguous back-and-forth.
You will regularly review code in domains you do not fully understand — payment processing, cryptography, medical data, legal compliance. The right response is not to approve blindly or to refuse to review.
For unfamiliar domain code, shift your focus from "is the business logic correct" (you may not be able to judge) to "is the structure sound." Are errors handled? Are the inputs validated? Is the code testable and tested? Is there documentation for the domain assumptions embedded in the logic?
Flag what you cannot evaluate. "I cannot verify the correctness of the fee calculation — this needs a domain expert review" is a legitimate and valuable review comment. It keeps the PR from merging without the right eyes on it.
Perfect code does not ship. The question in every review is not "is this perfect?" but "is this better than what was there before, and is it good enough to merge?"
A useful mental model: separate correctness from completeness. Code that is correct but incomplete — missing tests, rough naming, a TODO comment — can often merge if the incompleteness is tracked. Code that is incorrect should not merge regardless of how complete it looks.
If you find yourself blocking a PR for weeks on style and naming, ask whether you are reviewing for quality or for control. Lengthy review cycles have a real cost: the author loses context, the branch diverges, and the team learns to write larger PRs to reduce review frequency. Timely, focused reviews produce better code over time than slow, exhaustive ones.