How to Review a Pull Request in Under 30 Minutes

Thirty minutes is not enough time to review a pull request properly. That is the uncomfortable fact sitting underneath the entire genre of advice about reviewing pull requests quickly. The number is arbitrary, imposed by a manager or a sprint ritual or a vague sense that reviews are taking too long, and the person handed that number usually responds by skimming faster, which is exactly the wrong adaptation. A rushed review is worse than no review at all, because it puts a stamp of approval on code nobody actually read.

So the honest version of this subject starts with a reframe: the goal is not to read faster. The goal is to spend the thirty minutes on the parts of the change that carry risk, and to skip the parts that do not. Most pull requests contain a large volume of low-stakes material and a small amount of genuinely dangerous material. A reviewer who can tell the difference can finish in half an hour and still catch the things that matter. A reviewer who reads every line at uniform speed will miss them and feel productive doing it.

The first two minutes decide whether the review is even possible

Open the pull request and read the description before touching the diff. If the description says "fixes stuff" or is empty, close the tab and send it back. A change with no stated intent cannot be reviewed, because there is no standard to measure it against. The author has to say what problem this solves and roughly how. That is a two-sentence requirement, and it is non-negotiable.

If the description is adequate, read the title and the description together and form a hypothesis about what the diff should contain. A pull request titled "Add rate limiting to the search endpoint" should contain a limiter, a configuration value, some tests, and probably a metrics hook. That hypothesis is the tool that makes the next twenty-eight minutes efficient, because every part of the diff gets checked against it.

This is also the moment to check the size. A diff of forty lines can be read completely in a few minutes. A diff of two thousand lines cannot be reviewed in thirty minutes by anyone, and pretending otherwise is a lie told with a green checkmark. The correct response to an enormous pull request is to ask for it to be split, and to say so immediately rather than after twenty minutes of fruitless scrolling.

Read the tests first and the implementation second

Tests are the cheapest way to understand what a change is supposed to do. Skim them before the implementation. A good test file reads like a specification written in the author's own words: this input produces this output, this edge case raises this error, this dependency failing causes this fallback. Reading the tests first means the implementation can be read as an answer to a question already understood, which is dramatically faster than reading code cold and trying to infer its purpose.

Tests also reveal what the author was worried about. If the change touches a payment path and the tests cover the happy path and nothing else, that absence is a finding worth raising. If the tests are thorough and specific, the reviewer can move through the implementation with more confidence and spend the saved time on the parts of the diff that tests cannot cover, like naming, structure, and whether the change belongs here at all.

Watch for tests that assert nothing meaningful, or that mock so heavily they only verify the mocks were called. A test suite that passes when the implementation is deleted is worse than no tests, because it manufactures false confidence. That failure mode is common enough that a reviewer should look for it specifically rather than assuming tests mean coverage.

The diff view is the wrong place to read code

Reading a diff line by line is how reviewers miss things. A diff shows changes in isolation, stripped of the surrounding function, the class it lives in, the caller three files away. A line that looks fine in a diff can be obviously wrong in context, and a line that looks wrong in a diff can be perfectly correct given a guard clause twenty lines above it.

The efficient habit is to open the full file for any hunk that touches control flow, error handling, or anything with a conditional. Read the function as it now exists, not as a set of plus and minus lines. This costs a few extra clicks and saves the embarrassment of commenting on a bug that a surrounding block already prevents.

For large mechanical changes, the opposite applies. A rename, a formatting pass, a dependency bump: read the diff summary, confirm the shape matches the description, and move on. Spending twenty minutes scrutinizing a thousand-line rename is a waste of the thirty minutes that could have gone to the one function where the author made a judgment call. Reviewers who treat every diff with equal suspicion finish late and still miss the real problem, because attention is finite and it was spent in the wrong place.

Four questions that catch most of what matters

With the tests understood and the implementation read in context, the reviewer has maybe fifteen minutes left. That is enough time to ask four questions of the change as a whole, and those four questions catch a surprising share of real defects.

First: what happens when the thing this code depends on fails? A database call, an HTTP request, a file read, a queue publish. If the failure path is unhandled or swallows the error into a log line nobody reads, that is a finding. Second: what happens at the edges? Empty input, a very large input, a null where a value is expected, a unicode string where ASCII was assumed. Edge handling is where most production incidents originate, and it is exactly what a hurried reviewer skips.

Third: is this change doing one thing? A pull request that fixes a bug and also renames a variable and also tweaks a config value is three changes wearing a trench coat. Each one is harder to review because the reviewer cannot tell which lines belong to which intent, and harder to revert because reverting the bug fix also reverts the rename. The right move is to ask for a split, even when the change is small.

Fourth: would a future reader understand this without the pull request? Comments, names, and structure all serve the person who arrives six months from now with no context. A clever one-liner that saves four lines and costs an hour of future confusion is a bad trade, and the reviewer is the only person positioned to say so before it merges.

Comment on the code, not on the person who wrote it

Review comments have a tone problem that is entirely avoidable. "This is wrong" reads as an attack. "What happens if the list is empty here?" reads as a question, invites a response, and often surfaces a constraint the reviewer did not know about. The question form is also more honest, because the reviewer frequently does not know the full context and should not pretend to.

Reserve firm language for things that are actually firm: a security hole, a data loss path, a broken contract with another service. Everything else is a suggestion, and should be labeled as one. Mixing the two destroys the signal. A reviewer who blocks on variable naming teaches the author to ignore blocks, and the next block, the one about the unvalidated input, gets argued with instead of fixed.

There is also a category of comment that should never be written: the one about a preference the reviewer cannot defend. If the reviewer would have written the loop differently but cannot articulate why the current loop is worse, the comment is noise. It costs the author time, invites a debate about taste, and delays the merge for no reason. Good reviewers delete these before posting.

Approve, request changes, or say nothing and come back

The thirty-minute budget forces a decision at the end, and the decision is not always approve or reject. There is a third option that reviewers underuse: leave the review open and come back when there is time to do it properly. A change that touches authentication or migrations or anything that runs once and cannot be undone deserves more than a rushed half hour, and pretending otherwise is how outages happen.

Saying "I need another pass at this tomorrow" is a legitimate professional move. It is slower for the author, who now waits, and that cost is real. But it is dramatically cheaper than a defect that reaches production because a reviewer rubber-stamped a migration script at 5:40 on a Friday. The thirty-minute target is a tool for the common case, and the common case is a small, well-described change with tests. The uncommon case should be treated as uncommon.

For the common case, the workflow is mechanical enough to fit in the time: read the description, check the size, read the tests, read the implementation in context, ask the four questions, write comments as questions, and decide. A reviewer who does this consistently finishes most reviews in well under half an hour and catches more real problems than a reviewer who reads every line at speed. The difference is not diligence. It is allocation: attention spent where the risk lives, and skipped where it does not.

The last thing worth saying about the thirty-minute review is that it is a skill built by repetition, and the first dozen attempts will run long. The description check will feel slow. The context-reading habit will feel like extra work. After a month of doing it deliberately, the pattern recognition takes over and the whole sequence compresses. The reviewer who once needed fifty minutes to read a diff will find the same diff takes twenty, and will have more to say about it than before.