Read a pull request in this order: the description, the tests, the main change, the places where outside data enters, and then everything else. Starting with intent and tests tells you what the code should do, so deviations stand out. Reading files alphabetically buries the risky ones.
Why order matters
Most review tools list files alphabetically or by path. That order has nothing to do with risk. A reviewer who reads top to bottom spends their sharpest attention on the first files, which are often configuration and tests.
Google's public guidance on navigating a change recommends a sequence: take a broad view first, examine the main parts of the change next, and then go through the rest in an appropriate order. It also suggests that sometimes it helps to read the tests before the main code, because then you have an idea of what the change is supposed to be doing. That advice fits security review well.
The five steps
1. Description and purpose. Does the change make sense? Google's guidance says that if a change should not have happened at all, say so immediately and, ideally, suggest an alternative. For security, ask what it adds: a new endpoint, a new permission, a new place where outside data enters.
2. Tests. Read them before the code. They show the author's intent and, importantly, what the author did not think to test. Is there a test where a user asks for something that is not theirs? Is there a test for invalid input?
3. The main change. Find the file with the most logical changes. Google's guidance says to look at it first because it gives context for the smaller parts, and that significant design problems should be reported at once, even before you finish the rest. For security this is where you ask whether authorization, validation and error handling are all present.
4. Entry and exit points. The OWASP Secure Code Review Cheat Sheet recommends tracing data: identify sources such as user input, file uploads, API calls, database reads and environment variables, then follow processing, and check sinks such as database queries, file writes, output rendering, logging and external calls. In a diff, search for each new source and each new sink.
5. Everything else. Configuration, dependency changes, generated files and the long tail. Skim them with specific questions: did a permission widen, did a dependency appear, did a secret get committed?
If the diff is too large to order
Google's guidance says that if you cannot tell which parts are major, ask the author what to look at first, or ask them to split the change. For a security review, a good reply is: "Which three files hold the logic that touches input or permissions?"
Common traps
- Skipping removed lines. A deleted check does not show up in a search for new code. Read the red lines in the main files deliberately.
- Trusting the filename. A file called
helperscan hold the authorization logic. - Reviewing just what the diff shows. If a function is called but not changed, open it. Context outside the diff is where many gaps hide.
A printable version
1. Why does this change exist? What does it add?
2. Do the tests cover a bad input and a wrong user?
3. Does the main file validate, authorize, and handle errors?
4. Where does new data enter, and where does it land?
5. Anything in config, dependencies or deletions?
Five questions, about ten minutes on a small change, and a habit that scales to large ones.
Quick check
Should I read tests before code every time?
Google's guidance says it is sometimes helpful. Use it when the purpose of the change is unclear, and for security, whenever the change touches permissions.
What if the author did not write tests?
Say so, and ask for at least one test of the unhappy path before approval on security sensitive changes.
Sources
Last reviewed: 19 October 2026.
