In a code-review interview round, how should you prioritise the comments you raise on an unfamiliar change?
answer
- Outside in, not inside out
- Correctness before cosmetics
- Label each remark by weight
- Ask, do not accuse
- Finish with an explicit verdict
basics
~20 sWork outside in: correctness and data-safety defects first, then missing tests and unhandled edge cases, then interface and naming clarity, and only then style. Say the pass order aloud, phrase uncertainty as a question, and finish with an explicit verdict.
solid answer
~50 sA code-review round gives you a change someone else wrote — usually with one or two defects planted in it — and scores whether you find what matters and how you say it. Announce your pass order first so the interviewer can follow: correctness and anything that could lose or corrupt data, then tests and edge cases, then the interface and naming a future reader depends on, then style. Distinguish the three kinds of comment as you go — a blocking defect, a suggestion, and a nit — and label them, because an unlabelled pile of remarks makes the author guess what actually stops the merge. Phrase things you are unsure about as questions rather than accusations; you do not know the codebase's history. Name at least one thing the change does well. Close with a clear verdict and the conditions attached to it, rather than trailing off after the last remark.
go deeper
Learn the pass order and use it: correctness, then tests, then design and naming, then style. Practise by reviewing open changes in a public project and writing the comments you would leave.
Be ready to justify the ordering and to label each comment as blocking, suggestion or nit. Show that you can find a missing test case, not only a defect in the lines that were written.
Demonstrate the conduct half: asking instead of accusing in code you do not know, holding a position with a concrete failing case when challenged, and keeping scope discipline about pre-existing problems.
Own the tradeoff a review standard makes: a stricter bar catches more defects but slows delivery and pushes authors toward larger, less reviewable batches. Be able to say what your team blocks on and what it deliberately lets through with a follow-up.
## What this round is testing In a code-review round the interviewer hands you a change — a set of modified files, usually presented as a proposed contribution to a codebase you have not seen — and asks you to review it aloud, often inside 40 minutes or so. Two things are being scored at once: **detection**, whether you find the real defects, and **conduct**, whether your review is one an author would actually want to receive. Teams run this format because review is a large fraction of a working engineer's week and because it exposes judgment that a coding exercise does not: what you consider important, and how you behave toward someone else's work. Seeded reviews are usually stacked deliberately. A typical exercise contains one or two genuine correctness defects, a missing test or an untested edge case, a naming or interface choice that will confuse the next reader, and a scatter of cosmetic issues placed there to see whether you spend your time on them. ## The pass order Say it out loud before you start, then follow it: 1. **Correctness and safety.** Does it do what it claims? Data loss, silent failure, an unhandled error path, a boundary that is off, a concurrency assumption that does not hold, an input that reaches somewhere it should not. 2. **Tests.** Is the new behaviour covered? Does a test exist that would fail if the change were reverted? Are the interesting cases — empty, boundary, duplicate, out-of-order — represented? 3. **Design and interface.** Is the seam in a sensible place? Will the names still make sense to someone reading this in six months? Is a concept leaking across a boundary? 4. **Style and cosmetics.** Mention that formatting and lint belong to tooling, and keep this pass short. Most candidates who fail this round do so by starting at step 4. Spending the session on spacing and variable names while a boundary condition sits undetected reads exactly like it would on a real team. ## Comment types, labelled Mark each remark as blocking, suggestion, or nit. It costs you nothing and it tells the author what to do: | Type | Meaning | Example shape | |---|---|---| | Blocking | Merging this as written is a defect | This drops records that arrive after the window closes | | Suggestion | Better, but not a barrier | Extracting this branch would make the retry path readable | | Nit | Cosmetic, take it or leave it | Naming is inconsistent with the neighbouring function | ## Conduct Ask rather than accuse when you are unsure: 'is the late-arriving record handled somewhere upstream, or does it fall through here?' is stronger than 'this is broken', because in an unfamiliar codebase you may well be wrong, and the question still surfaces the issue. Comment on the code, never the author. Say what is good — a review that is only negative is less useful and reads as posturing. And when the interviewer defends a choice, engage with the argument; changing your mind on evidence is a positive signal, while conceding instantly to an interviewer who is testing you is not. ## A concrete picture At a developer-tools vendor selling a build-analytics service, a senior test engineer shares a change that adds a retry to the ingest path. A strong candidate reviews it in the order above and lands 6 comments: one blocking (retries are not idempotent, so a duplicated record inflates the totals), one about a missing test for the duplicate case, two suggestions about the boundary of the retry window and an unhandled error path, and two nits explicitly marked as such — plus a note that the extracted helper is a genuine improvement. Then a verdict: not as written, and here is the one thing that has to change. A weak candidate produces 19 remarks, all cosmetic, misses the duplication entirely, and offers no verdict at all. ## Closing End with a summary the author could act on immediately: the verdict, the one or two things that must change, the things you would like but would not block on, and any question whose answer would change your assessment. That summary is often what the interviewer writes down.
- The interviewer defends a choice you flagged as a defect. How do you handle that?Ask what you are missing and listen to the argument on its merits. If their explanation resolves it, say so plainly and move on — updating on evidence is a strong signal. If it does not, restate the specific failing case rather than the general objection: a concrete input and the wrong output it produces is much harder to hand-wave past than an opinion about the design.
- How much of the surrounding code, outside the change, is fair game to comment on?Briefly, and clearly marked as out of scope. Pre-existing problems are worth naming once — especially if the change makes one worse or depends on it — but asking the author to fix code they did not touch is how reviews stall in real teams. Say which of your comments are about the change and which are about the neighbourhood.
saying these in an interview costs you the question
- Opening with formatting and naming while a correctness defect goes unfound
- Producing a long undifferentiated list with nothing marked as blocking
- Phrasing remarks as attacks on the author rather than the code
- Never stating a verdict, so nobody knows whether it can merge
- Conceding a real defect the moment the interviewer pushes back