Nobody teaches you code review. You learn it by doing it wrong, getting weird looks in standups, and slowly absorbing the unspoken rules through years of awkward pull request comments.
Here are the rules nobody writes down but every experienced developer knows.
Rule 1: Review the PR, Not the Person
There's a difference between:
- "This function is poorly named"
- "What do you think about renaming this to
calculateTotalPrice? It'd make the call sites easier to read"
The first judges. The second collaborates. Same feedback, completely different energy.
Use "we" and "what if" language. You're working together to make the code better, not grading someone's homework.
Bad: "You should have used a map here instead of a for loop."
Good: "A map() might be a nice fit here — it makes the transformation more explicit."
Also good: "Nit: could use map() here" (the "nit" prefix signals this is minor)
Rule 2: Approve With Comments
The most anxiety-inducing experience in a new developer's career is opening a PR and seeing 47 comments. Even if 45 of them are suggestions and nitpicks, the wall of text feels like failure.
If the code works, the approach is sound, and the issues are minor — approve with comments. This tells the author: "This is good to go. Here are some thoughts for this PR or future work."
Reserve "request changes" for:
- Bugs that will break production
- Security vulnerabilities
- Architecture decisions that need discussion
- Missing tests for critical paths
Everything else? Approve and note it.
Rule 3: Nits Are Fine. Nit Avalanches Are Not.
A "nit" is a minor suggestion — formatting, naming, a slightly more idiomatic way to write something. Nits are fine individually. But 20 nits on a single PR feels like death by a thousand paper cuts.
The rule: If you have more than 5 nits, pick the top 3 that matter most and skip the rest. Or batch them into one comment: "A few small nits — take 'em or leave 'em."
Better yet, set up a linter and formatter so machines handle the nits. Nobody's feelings get hurt by an ESLint error.
Rule 4: Ask Questions Instead of Making Statements
Questions open dialogue. Statements close it.
Statement: "This should use a cache."
Question: "Have you considered caching this? It gets called on every request and the data doesn't change often."
The question gives the author space to say "Actually, we need fresh data here because..." — which might teach YOU something. The statement just implies they're wrong.
The exception: if something is clearly a bug, say so directly. "This will throw a null pointer if user is undefined" isn't the time for Socratic dialogue.
Rule 5: Big PRs Are Everyone's Problem
You open a PR. It has 47 files changed, 2,000 lines added. Your reviewer looks at it, sighs, scrolls through the diff for 5 minutes, and approves with a comment that says "looks good" — because nobody is going to thoroughly review 2,000 lines.
The unwritten rule: If your PR is too big to review properly, it's your job to break it up, not the reviewer's job to power through it.
Good thresholds:
- Under 200 lines: Easy to review thoughtfully
- 200-400 lines: Reviewable but needs focus
- 400+ lines: Consider splitting
If a big PR is unavoidable (migration, generated code), add a description that guides the reviewer: "The interesting changes are in auth.ts and middleware.ts. Everything else is generated types."
A good text diff tool helps here too — sometimes pasting old and new versions side by side reveals the meaningful changes faster than scrolling through a giant PR diff.
Rule 6: "LGTM" Is a Valid Review (Sometimes)
"Looks Good To Me" gets a bad reputation. But if a senior developer who knows the codebase glances at a small, well-tested PR from a trusted colleague and says "LGTM" — that's a perfectly valid review.
Context matters:
- Small fix to a well-tested area by a trusted contributor = LGTM is fine
- New feature, complex logic, or unfamiliar area = give it the full review
- Junior developer's code = always leave substantive feedback (they're learning from your reviews)
Rule 7: Review Speed Matters More Than Review Depth
A controversial one. A PR that sits unreviewed for 3 days is worse than a PR that gets a "good enough" review in 3 hours.
Why:
- The author's context fades. By day 3, they've moved on mentally and switching back is expensive.
- It blocks other work. Stacked PRs pile up, features stall, and momentum dies.
- It demoralizes the team. Nothing kills motivation like watching your PR sit in limbo.
The rule: Aim to review within a few hours, not a few days. A quick review that catches the big stuff is better than a perfect review next week.
If you're too busy, say so: "I can't review this until tomorrow — if someone else can grab it, go ahead."
Rule 8: Don't Bikeshed
Bikeshedding is spending 45 minutes debating whether a variable should be called userData or userInfo while a real bug ships to production.
If a naming or style choice doesn't affect correctness, readability, or maintainability in a meaningful way — let it go. Save your review energy for things that matter: logic bugs, missing edge cases, security issues, and architecture decisions.
If you find yourself typing the third reply in a comment thread about bracket placement, stop. One of you is wrong and it doesn't matter which one.
Rule 9: Context Is King
"Why are we doing this?" is the first thing a reviewer should understand, and it's the author's job to make it clear.
A good PR description includes:
- What: What does this change?
- Why: What problem does it solve?
- How: How does the approach work? (for non-obvious implementations)
- Testing: How was it tested?
A diff without context is just a puzzle. Nobody wants to reverse-engineer the motivation from 30 file changes.
Rule 10: Your Style Preferences Aren't Universal Laws
You prefer forEach over for...of. You think ternaries are always more readable than if/else. You believe single-letter variables are fine in short lambdas.
These are preferences, not rules. Unless your team has a documented style guide that agrees with you, don't push your preferences in reviews.
The test: If you can't explain WHY something is better (beyond personal taste), it's a preference, not a code quality issue.
Rule 11: Leave at Least One Positive Comment
Code review is inherently negative — you're looking for problems. But the author just spent hours (or days) on this work. One comment like "Nice approach here" or "Smart use of the cache" costs you 5 seconds and completely changes how the rest of the feedback lands.
This isn't about being fake. Find something genuinely good and point it out. If nothing stands out, "Clean implementation, just a few thoughts below" sets a collaborative tone.
Rule 12: The Author Has the Last Word
You suggested a change. The author considered it and disagreed. Now what?
Unless it's a correctness or security issue, the author gets the final call. They have the most context on the problem they're solving. Your job as a reviewer is to raise concerns, not dictate solutions.
If you truly believe the approach is wrong, escalate to a team discussion — not a 15-comment thread on a PR.
The Meta-Rule
Code review is a human activity with technical content. The technical part is easy — does the code work, is it correct, is it maintainable? The human part is hard — giving feedback that's honest without being harsh, timely without being sloppy, thorough without being pedantic.
The best code reviewers aren't the ones who catch the most bugs. They're the ones whose feedback makes people want to write better code.