A code review that helps a junior is a teaching conversation with a diff attached. The reviewer reads for intent first, explains the reason behind every request, and finishes by asking the author to walk through their own change. That extra step costs fifteen minutes and is usually the highest-leverage fifteen minutes in a new hire’s first quarter.
Most reviews go wrong in one of two directions. Either the reviewer corrects the code line by line and the junior learns nothing beyond how to satisfy the linter, or the reviewer approves a large diff with three polite comments and no real feedback at all. Both feel productive. Neither teaches.
The rest of this guide is the flow I use: what to prepare, a seven-step review, and the specific comment rewrites that change how a junior reads feedback. It takes about 60 to 90 minutes of reviewer attention for a normal-sized change, and it works for volunteer civic data projects just as well as it does for a commercial team.
Table of Contents
- What You Need
- Step-by-Step
- 1. Set a Clear Learning Goal for the Review
- 2. Inspect the Change Before Asking Questions
- 3. Review Correctness, Design, and Readability
- 4. Write Comments That Explain the Reason
- 5. Discuss the Review in a Small Conversation
- 6. Verify the Revision Without Starting Over
- 7. Close the Learning Loop
- Common Mistakes
- Frequently Asked Questions
- How long should a code review take for a beginner’s pull request?
- How do you give code review feedback to a junior without sounding condescending?
- Should junior developers review senior engineers’ code?
- What code review metrics should you track?
- Can ChatGPT or an AI bot do a code review?
What You Need
Before anyone types a comment, five things have to exist. On a team where these are missing, the review turns into a live debugging session, and that is the format juniors learn least from.
- A diff small enough to hold in your head. Under 400 changed lines is a reasonable ceiling for a first pass. Bigger diffs get skimmed, and skimmed reviews produce rubber-stamped merges.
- An author note. Three sentences at the top of the pull request: what changed, why this approach, and what you are unsure about. Reviewers who understand the intent catch different defects than reviewers who only see the code.
- Green tests before a human opens it. If CI is red, the review has not started yet. Asking a senior to debug a failing build through comment threads is a waste of the one scarce thing you have, which is the senior’s attention.
- A named reviewer who fits the author. Not the person who happens to be online. Routing matters, and the routing rules are in step one.
- A shared severity vocabulary. Everyone uses the same four labels so the author can triage without decoding your mood: blocker, suggestion, question, nit.
That last item does more work than it looks. A junior who can tell a blocker from a nit stops rewriting the whole file to satisfy a formatting opinion, and stops opening a pull request two weeks late because they were dreading the response.
Step-by-Step

1. Set a Clear Learning Goal for the Review
Decide, before you open the diff, what this author should walk away knowing. Not everything. One or two things is realistic in a single review.
The goal should be observable so you know whether it happened. “Learn about testing” is vague. “Write a test that covers the empty result branch, and explain why that branch needed one” is something you can check when the revision comes back.
Common goals map cleanly to the kind of change in front of you:
- Naming: does a function name tell you what it does without reading the body?
- Error handling: does the code fail loudly and locally instead of swallowing an exception?
- Testing: is there a case that would fail if the logic broke?
- API design: does the new interface make the wrong thing hard to do?
- Readability: would a stranger understand this in six months?
State the goal in the pull request as your first comment. Something like: for this one I care most about error handling and naming, so expect more comments there and almost none on formatting.
2. Inspect the Change Before Asking Questions
Read the ticket, the acceptance criteria, and any architecture constraints before the diff. Then read the diff once without commenting at all, so your first reaction doesn’t set the tone for the whole thread.
Check what changed outside the obvious files. New dependencies, a modified migration, a config change buried three folders deep. Those are where the expensive surprises live.
If the diff is far past 400 lines, stop and ask for a split before reviewing. The split is itself a lesson: a 1,200-line pull request means three reviewable changes were mixed into one.
3. Review Correctness, Design, and Readability
Run the passes in this order. Correctness first because a naming comment on a function that returns the wrong value is noise, and design before readability because renaming an interface later costs ten times what naming it right costs now.
- Correctness. Does it do what the acceptance criteria say? Boundary conditions, empty inputs, timezone and encoding handling, what happens on the second call.
- Data and security risk. Unvalidated input, secrets in configuration, a query that will not hold up at ten times current volume, a permission check that lives in the UI and nowhere else.
- Design. Does this fit the pattern the codebase already uses, or has the author quietly invented a second way to do the same thing?
- Tests. Is there a test that fails if the change is reverted? That single question filters out most test-shaped code that proves nothing.
- Readability. Long function, deep nesting, a name that lies.
- Polish. Formatting, comments that explain why rather than what. Only after the rest, and only as nits.
Keep a rough inspection rate in mind. Somewhere around 300 to 500 lines per hour is a realistic pace for careful reading. If you blow past that, you are not reviewing, you are scanning, and the quality falls off a cliff.
4. Write Comments That Explain the Reason
This is where most junior reviews fail, and it has almost nothing to do with technical accuracy. A comment that says what is wrong teaches nothing. A comment that says what is wrong, why it matters here, and one direction forward gets remembered a year later.
| Instead of | Write | Why it lands |
|---|---|---|
| “Use a better variable name.” | “I read x three times before finding the unit. Could this be tripStartTime? Not a blocker.” | Shows your reading, gives a concrete option, and labels the severity so the author knows it is optional. |
| “This is wrong.” | “If getUser() returns nothing, this line throws. What should the screen show in that case?” | Describes the failure you actually saw and turns a statement into a question the author can answer. |
| “Why did you do it this way?” | “I would not have expected a queue here. What problem does this solve that a direct call does not?” | Removes the implication that they did something dumb. New juniors often read a bare why-question as an accusation. |
| “Add error handling.” | “If this fetch fails the page renders empty and the user sees a blank screen. Let us decide what they see.” | Points at the user-visible failure instead of naming a pattern. |
| “This is not how we do things here.” | “We usually map over an array in the view model rather than in the template, so controllers stay thin. That is a convention, not a law here.” | Separates a real standard from a personal preference, which is a distinction juniors cannot see on their own. |
| “Clean this up.” | “Two of these five branches do the same thing. Can we collapse them, or is the difference deliberate?” | Asks instead of assigns, and gives them the chance to explain a reason you do not have yet. |
| “Remove this dead code.” | “Is legacyFlag still read anywhere? If not, deleting it here saves us a confusing grep later.” | Scopes the request to this change instead of turning it into a side quest. |
| “Tests missing.” | “Nothing here fails if we change the rounding. Can we add one case with 0.5 so this behavior is pinned?” | Names the exact missing case so the test is a two-minute job. |
Use labels. Blocker: must change before merge, correctness, security, or data risk. Suggestion: I would change this, here is why. Question: I do not understand this yet, please explain. Nit: taste, and I will approve either way.
The question label does the most work. A reviewer who is confused should say so instead of guessing, because a guessed answer produces a confident fix and a confident fix is how bad patterns spread.
5. Discuss the Review in a Small Conversation
Written comments are the wrong tool for anything where the reasoning is the point. If a thread has gone past five comments on the same topic, or the author has pushed three revisions without absorbing the feedback, move to a fifteen-minute screen share.
Structure the call so the author does the talking. Ask them to walk through their design decision first, then ask why. Do not open by telling them what you would have done.
One pattern works well when a junior keeps missing the same idea: write the better version once, together, in a scratch branch, and talk through why it is better. Senior engineers on forums consistently say this teaches more than ten rounds of comment arguments, and it keeps the knowledge inside the author’s head instead of the reviewer’s.
Watch for the other failure mode: taking the keyboard and finishing the change yourself. That is faster today and teaches nothing. Write the file for them and they will write the next one the same way.
6. Verify the Revision Without Starting Over
On the second pass, do not reread the whole diff with fresh eyes. You already know what you are looking for. Check the specific items you raised, plus whatever came along for the ride.
Run the tests yourself once. A junior who believes a fix works because it looks right is a junior who ships a broken branch at 5pm.
When you disagree and the author pushes back, do not settle it by approval rank. Ask what would have to be true for their approach to be safe, then check whether that is true. If neither of you has the answer, convert the disagreement into a small experiment or a note in the pull request, and say plainly that you are not sure.
Say it out loud when you do not know. Juniors calibrate their entire model of the work on whether senior engineers admit uncertainty.
7. Close the Learning Loop
Most reviews end at approval, which is the point where the learning would have happened. Spend two minutes on the close instead.
- Summarize what changed in their revision, in one sentence: you split the error handling out of the view model, which makes it testable.
- Name one improvement you noticed. Specific, recent, and real. Not great work, nice effort.
- Assign one follow-up exercise before they move on. Read one function in the codebase and write down why it is shaped that way. Or pair on the next risky change.
- Note a pattern for their next review. If naming came up three times in a month, that is a coaching item, not a character flaw.
Keep these notes somewhere the author can read them later. A shared doc of growth notes does more for a junior’s confidence than any compliment inside a pull request.
Common Mistakes

These are the habits that quietly turn a review into a lesson in the wrong thing.
Fixing the code yourself. It is the fastest outcome for the team and the slowest outcome for the author. Hand the keyboard back, or open a scratch branch and write it together out loud.
Piling on nits after the real feedback. Once a reviewer lists twenty style notes, the three things that mattered disappear into the noise. Ship the nits as a follow-up list the author can do later, or run the formatter and move on.
Requesting out-of-scope refactors. Renaming a shared module because it is confusing is a separate change with its own pull request. Mixing it in teaches the author that review is arbitrary.
Reviewing every line of a huge diff. Nobody does it. Say the number out loud: this is 1,400 lines, split it into three pull requests and I will review each one properly.
Approving to unblock someone. A rubber stamp teaches the opposite of what you want. If you are too busy to review it now, say so and set a time. Approving unread code because a deadline is close is how the next incident gets built.
Using review metrics as a scoreboard. Defects found per developer is a terrible number for a person and a decent number for a process. Once juniors know their numbers are counted, they write small safe changes and hide the risky ones. Track review latency, re-review count, and how long comments sit unanswered. Never track individuals by comment volume or defect count.
Letting AI do the talking. Automated review bots and assistants are good at catching formatting, dependency smells, and missing null checks. They are bad at knowing which of those matters in your codebase this week. Worse, generated comments read as confident and generic, and juniors cannot tell that tone apart from a senior who genuinely means it. Use the bot for the mechanical pass, then review the output before it lands. If the author wrote their code with an assistant, ask them to explain any line they would not have written alone.
A few smaller habits worth keeping: praise something specific in the pull request before the first comment, review your own diff before requesting review, and keep review turnaround inside one business day. Most complaints about strict reviewers turn out to be complaints about slow ones.
Frequently Asked Questions
How long should a code review take for a beginner’s pull request?
Budget 30 to 45 minutes of your attention for a change under 200 lines, and up to 90 minutes for one that approaches 400. Beyond that, ask for a split rather than pushing through. The number that matters more is the limit on you: after roughly 90 minutes of reading, reviewers reliably start missing defects, and the author gets a review that looks thorough but is not.
How do you give code review feedback to a junior without sounding condescending?
Lead with what you understood, not with what is wrong. Ask questions where you are genuinely unsure, label every comment by severity so the author knows what must change, and explain the reason behind each request. The tone that lands is curiosity about the code plus certainty about the standard. Juniors read a bare why-question as an accusation even when you meant it as interest.
Should junior developers review senior engineers’ code?
Yes, with support. Have them start with a scoped review, such as tests or documentation, and pair with them on the first pass so they learn what senior code looks like from the inside. Avoid putting a brand-new hire as the only reviewer on a risky change. Reciprocity helps too: a junior who has reviewed with a senior tends to ask for review more often and earlier.
What code review metrics should you track?
Track review turnaround time, the number of re-review cycles a change needs, how long a thread sits unanswered, and the percentage of changes that reach a clean first approval. Those measure process health and they improve when the culture improves. Do not track defects found per developer or comment counts per person. Once those numbers reach a performance review, people stop writing risky code and stop asking for review.
Can ChatGPT or an AI bot do a code review?
For the mechanical pass, yes. Tools reliably catch formatting drift, unused imports, missing null checks, and dependency issues, and they do it in seconds. For intent, design fit, and whether the change actually solves the ticket, no. A bot cannot tell whether a strange workaround matches a constraint you would only know from a conversation last week. For a junior, an unreviewed bot pass is also a missed chance to practise reading code closely.
If you take one thing from this, do the loop properly on your next review: pick one skill to teach, write comments that explain the reason, and spend the last two minutes naming what improved. That closing habit is what separates a code review that helps juniors from one that only catches defects.


