How I Review PRs as an Engineering Manager

For the first year after I became a manager, I reviewed pull requests exactly the way I had as an engineer. I read the diff top to bottom, checked the tests, debated naming, and quietly kept track of all the ways I would’ve written it differently.

I could spend twenty minutes on a variable name and completely miss that nobody on the thread knew who would own the thing if it broke in production.

That happened more than once.

The review process at my company reinforced the habit. It was built by engineers for engineers going line by line, with the reviewer acting as the last line of defense before code shipped. That’s useful work, but somewhere in the move to management, it stopped being the most useful work I could do.

I was reviewing the implementation while missing the decision around it. Clean code could still shift risk onto another team. Great tests could still prove the wrong behavior. A tiny diff could still move real money or create a terrible day for a customer.

So I wrote down what I actually care about now. It came out to three moves: understand the so what, verify it against reality, and only surface what’s consequential.

My version of this defaults to unblocking. I’d rather a team ship and learn than wait around for me to invent a comment. Another manager might stay closer to the architecture or still spend a lot of time building alongside the team. They’d probably use the same moves and land somewhere stricter.

The altitude changes. The questions are still useful.

This is also only my pass as an EM. It doesn’t replace the engineer checking correctness, the code owner checking the design, or a security or privacy review when the change needs one. My job is to add the context I’m uniquely positioned to see, not cosplay as every reviewer at once.

---

Understand the So What

Before I read a single line of code, I ask what the PR is actually for.

What changes for a customer or the business once this ships? Who owns it after it merges? What happens if it’s wrong?

Those answers tell me how to read everything that follows. A one-line feature flag change for an internal tool is a different animal from a one-line change to how a background job retries a failed payment. The diff size tells me almost nothing about the stakes.

I want to name the outcome in one sentence. Maybe it’s a faster onboarding flow, a fix for a support escalation, or cleanup nobody asked for that saves the team pain later. If I can’t explain why the work exists, I don’t understand enough to approve it.

I want ownership to be boringly clear. Code doesn’t stop being anyone’s problem when it merges. Someone gets paged when it breaks at 2 AM, and someone has to explain it when a customer finds the behavior nobody expected. An incident is a pretty bad time to discover that everyone thought another team owned it.

I want to know how wrong this can go. How many people notice, how quickly do we notice, and can we roll it back? I don’t need every PR to carry an incident plan, but I do need the amount of care in the review to match the actual blast radius.

I’ve seen PRs where the implementation was genuinely excellent and none of this was answerable. That combination worries me more than an awkward method name ever will.

---

Verify Against Reality

Once I understand the story, I check whether the system tells the same one.

Trace the path at least one hop further. I follow the changed code into whatever it touches next, the adjacent service, the background job, the boundary where this system hands off to another one. If the risk keeps traveling, I keep following it. A change that looks self-contained in the diff view often stops looking that way the moment I follow it past its own borders.

Compare it to how the system already works. If three other parts of the codebase handle retries, timeouts, or permissions one way and this PR does something new, I want to know whether that was a decision or an accident. Different isn’t automatically wrong. Accidental is what worries me.

Check the edges. What happens when the request times out, when the job runs twice, when two people do the thing at almost the same moment. Most PR descriptions are written about the case that works, and that’s the case I trust the least, because nobody had to think hard to write it.

This is where I still get my hands dirty and read the actual code, just with a different question in my head. I don’t care whether I’d have written it this way. I care whether the PR’s story survives contact with the system around it.

---

Surface What’s Consequential

This was the hardest part to unlearn. More comments used to feel like more rigor. Now, a long review from me usually means someone else has to sort my preferences from the things that actually matter.

My bar includes money movement, customer harm, data integrity, security and privacy, ownership, and whether the thing can be operated and debugged later. If a concern is really about my taste and the team’s conventions allow it, I keep it to myself.

That means the naming preference stays in my head. So does the refactor I’d personally choose and the pattern I wouldn’t have used. The engineer closest to the work gets to own those decisions, including making some choices I wouldn’t make.

A real comment I’ve posted looks something like this:

question: who owns the retry behavior once this merges, us or the team that owns the downstream job?

That’s it. One question tied to a real risk, not a paragraph of me thinking out loud on someone else’s PR.

Most of the time, nothing consequential turns up, so I say that and approve.

Manufacturing a concern so I can feel useful is worse than saying nothing. It trains the team to expect friction that has nothing to do with real risk. It also makes the comment that does matter harder to recognize.

---

Building the Skill

Writing this down for myself helped. Writing it clearly enough that Claude Code could run it exposed how much of my review process still lived in vibes.

I had judgment calls I’d been making instinctively for years, but I couldn’t explain half of them on the first try. What felt like intuition was usually a principle I had repeated enough times that I’d stopped noticing it.

The skill is just a reusable set of instructions for Claude Code. It pulls the diff, drafts the so what, checks the code against the surrounding system, and only gives me comments that clear my bar. It doesn’t approve anything for me. I still read its evidence, check its assumptions, and make the call.

Here’s the generic version. The two bracketed sections are the parts worth changing because they force the actual questions: what altitude am I reviewing from, and what deserves my interruption? I keep it at .claude/skills/review-pr-as-em/SKILL.md in the project where I use it.

---
name: review-pr-as-em
description: Review a pull request from an engineering-manager altitude, outcome and ownership first, implementation second. Use when reviewing PRs as an EM rather than as the implementing engineer.
---

# Review PR as an EM

## My Altitude
[Fill in where you sit. Mine: I default to unblocking. I'd rather a team ship and iterate than stall waiting on a comment from me. An architect-leaning or builder-leaning EM would run the same three moves and land somewhere stricter.]

This EM review complements implementation, code-owner, and specialist reviews. It does not replace them.

## Move 1: Understand the So What
Before reading the diff, answer:
- What outcome does this unlock for a customer or the business?
- Who owns this after it merges?
- What's the blast radius if it's wrong?

If the PR description doesn't answer these, that's the first question, before anything in the code.

## Move 2: Verify Against Reality
Check the story against what's actually true in the system:
- Trace the changed code at least one hop past its own boundary, and keep following it when the risk travels further.
- Compare it to how the codebase already handles this elsewhere. Divergence isn't automatically wrong, but it should be intentional.
- Check whether the tests prove the intended outcome and cover the meaningful failure paths.
- Check the edges: timeouts, retries, double-runs, two people doing the thing at once.

## Move 3: Surface What's Consequential
Only raise manager-altitude concerns that touch [fill in your own bar. Mine: money movement, customer harm, data integrity, security and privacy, ownership, operability]. Leave personal implementation preferences unsaid when team conventions allow them.

Do not suppress correctness concerns or specialist review needs because they fall outside this bar. If required checks and reviews are complete and nothing consequential turns up in this pass, say so and approve. A manufactured concern trains people to expect friction that has nothing to do with real risk.

## Output
- The so what, one paragraph, including the evidence inspected and any assumptions or uncertainty.
- Zero or more comments, each anchored to one principle above and concise without omitting the evidence or impact.
- If nothing clears the bar: "Nothing here rises to [your bar]. Approving."

The agent saves me time, but the more useful change happened before I built it. I had to stop treating every PR as a chance to prove I could still be the engineer in the room.

I still get that urge sometimes. Now I have a better question than “how would I write this?”

What does this team actually need from me before this ships?