Code Review: How to Do It
8 Tricks to Keep Your PR From Getting Rejected
Code review isn't a "trial", it's a "teaching session". The person writing the PR shouldn't get criticized, and the person reviewing it shouldn't put on airs — when both sides do it right, code review becomes the cheapest channel there is for leveling up the whole team. Here are 8 principles each for the people writing PRs and the people reading them.
First, get clear on 3 things
01 You're reviewing the code, not the person
This sounds simple, but in practice 70% of conflicts come from blurring this one line.
- Right: "This code assumes the input is always an array and misses the null case."
- Wrong: "You didn't account for null — that's sloppy."
The difference: the first sentence describes the code, the second judges the person. Always talk about the code, never the person.
02 The reviewer isn't a gatekeeper, they're a collaborator
A lot of reviewers see themselves as "the one guarding the gate" — watching to see who can clear their bar.
A real senior reviewer sees themselves as "the person helping the author make this PR better". Same side, not opposite sides.
03 The author isn't being judged, they're asking for input
Opening a PR isn't "please everyone, just let me through" — it's "I built this, please help me check whether I missed anything".
Get the posture right and PR review turns into a productive conversation instead of political maneuvering.
For the people writing PRs — 8 habits that make reviewers want to approve
01 Write the PR title as "action + scope"
❌ Reviewer wants to close the tab
fix bugupdate fileWIPmore changes
✅ Reviewer wants to click in
fix(auth): handle expired token in middlewarefeat(payment): add Stripe webhook for refundsrefactor(order): extract pricing logic to service
The PR title is the first thing a reviewer sees, and it signals whether you put in the effort.
02 Write the "why" in the PR description, not just the "what"
A solid PR description template:
## Why [Why is this change needed? Background, pain point, related ticket] ## What [What changed? High-level description, no need to list every file] ## How [Key design decisions, especially "why A instead of B"] ## Testing [How did you test it? Which edge cases are covered / not covered] ## Screenshots (if UI) [before / after comparison] ## Reviewer hints [What should the reviewer pay special attention to? Where are the trade-offs?]
Spend 5 minutes on this description and you'll save 30 minutes of "why?" back-and-forth later.
03 One PR does one thing
The classic bad PR: "refactor auth + fix a payment bug + add a new API + bump deps" — the reviewer has no idea where to look.
A good PR: one clear topic. Even if you happened to fix something small along the way, put it in a separate commit so the reviewer can see it.
Rule of thumb: one PR should be no more than 400 lines of changes. More than that, split it into several.
04 Review it yourself first
Before opening the PR, read through it yourself in GitHub's "Files changed" tab.
You'll find:
- a leftover
console.logyou forgot to delete - an uncleaned comment like
// TODO fix later - test data you committed by accident
- an obvious typo in a name
Catch this small stuff yourself; don't make the reviewer catch it. When a reviewer finds these, it lowers their estimate of "how much care you put in overall".
05 Give the reviewer a "guided tour"
When the PR is big, use inline comments to flag the key parts yourself first:
// Inline comment on the PR: "@reviewer This block is the new pricing logic, extracted out of OrderService. The logic itself didn't change, I only changed how dependencies are injected."
This is "saving the reviewer time" — every senior engineer does it.
06 Don't push back on every suggestion
Some authors' first reaction to a comment is to "explain why they wrote it that way".
That's fine, but it depends on the situation:
- The reviewer is clearly right → just change it, don't argue
- There's a design trade-off to explain → write 1–2 sentences
- You genuinely disagree → lay out the trade-offs and invite discussion
If you push back on every single comment, the reviewer won't want to review your next PR.
07 Don't make the reviewer wait on a failing CI
A common bad habit: you open a PR, CI goes red, you don't fix it, and you pass the buck to the reviewer with "can you take a look for me".
Senior engineers — when CI goes red, fix it yourself first. Once CI is green, then ping the reviewer.
This is respecting the reviewer's time.
08 Keep tracking it even after it's approved
A lot of people forget about the PR the moment it's approved.
Senior engineers:
- watch the production deploy after merge for any issues
- if there's a metric, check whether the KPI moved
- circle back a week later — did this change actually achieve what it set out to do?
This is "owning your code all the way through", not "it's done once I hand it to the reviewer".
For the people reading PRs — 8 reviewer principles
01 Before reading the PR, ask "what is it trying to solve"
Don't open the PR and jump straight into the code diff.
The order:
- Read the PR description — what is it trying to do?
- Read the related ticket / spec (if there is one)
- Then look at the code diff
Read the diff with no context and you'll end up reviewing trivial things while missing the real problem.
02 Go from the big picture down to the details
The review order:
- Architecture / design — is this PR's approach right?
- Boundaries / interfaces — are the new functions / APIs named clearly with clear responsibilities?
- Logic — are there bugs in the implementation? Are edge cases handled?
- Tests — are there corresponding tests?
- Details — naming, code style, comments
Flip the order and you'll spend 30 minutes nitpicking names, then realize the whole approach is wrong and it all has to be redone.
03 Use a "nit" prefix to mark non-blocking opinions
Not all opinions carry equal weight.
// Examples nit: this variable name would be clearer as userCount suggestion: consider extracting this into a helper function question: what alternatives did you weigh for this trade-off? blocking: this query is N+1 and will blow up in production
- nit: a small thing, the author can choose whether to change it
- suggestion: a recommendation, doesn't block merge
- question: asking a question, not requesting a change
- blocking: must be changed before it can merge
This way the author knows "which ones must be changed, and which can go in the backlog".
04 For every comment, write "why you're suggesting the change"
❌ Nobody learns anything
"This is poorly written""Using for here is bad"
"This is too complex"
"This is wrong"
✅ The author gets it next time
"The if nesting here goes more than 3 levels deep — extract a helper function to keep the logic linear""Swap the for + push for map; it expresses the intent of 'transforming an array' more clearly"
"You've got 4 states depending on each other right now; switching to useReducer to centralize them would be easier to maintain"
Every comment should at least include: what the problem is + how you suggest fixing it + why.
05 Find 1–2 things worth praising
This is the most underrated review technique.
Authors are nervous when they open a PR (afraid of being criticized). Leaving 1–2 positive comments — "this split is really clean", "this name is spot on", "this test case is really thorough" — makes the author far more willing to take your criticism.
This isn't "empty politeness", it's "psychologically effective".
06 Leave one "I learned something"
Find the spot in the PR that made you go "huh, that's a nice way to write it" and leave a comment:
"I didn't realize Promise.allSettled could be this clean in this scenario — learned something."
Two effects:
- The author feels validated and is more willing to take your reviews next time
- You'll actually remember this pattern and use it yourself later
07 When you're unsure, ask — don't pretend to understand
When you don't understand a piece of code, the worst move is to "pretend you do and approve".
The right move: leave a question comment:
"I didn't follow the timing here — why 200ms? Is there a spec requirement?"
Asking questions isn't embarrassing. Pretending to understand means you share the blame when things break.
08 Don't be the last line of defense
The responsibility reviewers most easily overload onto themselves: feeling like "I have to catch every bug, or it's my fault once it merges".
Wrong.
A reviewer can't replace tests, can't replace CI, can't replace monitoring. Your job is to "do your best to help, not to guarantee 100%".
If your team dumps all responsibility for quality on the reviewer — the process is broken, and that's not on you.
Handling disagreements — 3 principles
01 Distinguish "principle" from "preference"
- Principle: things that break if violated (security, performance, tests) → don't give ground
- Preference: you like A, they like B, and neither is wrong → the author decides
Most disagreements are "preference" — the reviewer likes functional, the author likes imperative. There's no right or wrong here, so let the author choose.
02 When words aren't cutting it, get out of text
PR comments have gone back and forth 5 times with no consensus — just set up a 15-minute voice call.
90% of disagreements are really "text communication falling short", not a genuine difference of opinion. Five minutes on a call settles it — don't spend a whole day arguing on GitHub.
03 When you truly can't settle it, bring in a third party
Two people each holding their ground, neither able to convince the other — bring in the tech lead, or a team senior, to make the call.
The point: find someone to make the call, not "whoever's loudest wins".
Code Review in the Age of AI
In 2025–2026, AI code review tools (CodeRabbit, GitHub Copilot Review, Cursor Review) keep getting stronger.
What AI can do
- catch lint / type / obvious bugs
- suggest naming and code style
- flag potential null / undefined
- check test coverage
What AI can't do
- judge "is this architectural choice right"
- judge "is this trade-off right for our team"
- know "how a similar feature was done last time"
- provide "political context" (will this change affect another team?)
Suggested way to combine them:
- PR is opened — AI auto-reviews first, catching nits / obvious bugs
- The author does one pass based on the AI feedback
- The human reviewer steps in and focuses on architecture, design, and business context
This way the human reviewer doesn't waste time nitpicking semicolons / names and can focus on the value "AI can't see".
One last reminder
The long-term value of code review isn't "catching bugs", it's "passing on the team's engineering culture".
From the way you review, new hires learn what the team cares about, what it dislikes, what it aspires to.
A team's level — is often exactly the level of its PR comments.
That's also why senior engineers spend so much time reviewing — it's the way to pull the whole team up to your level at the lowest possible cost.
Next time you review someone's PR, don't just think "are there bugs" — think "this comment, when a new hire reads it 3 years from now, what will they learn?".
Stuck on a code review and want someone to sanity check it?
A 30-minute 1-on-1 consult for NT$1,500 — I'll mock-review a recent PR of yours and give you concrete directions to improve.