Dobro! You’ll rarely meet a developer (and if you do, he’s probably an 👽) who hasn’t done a pull request review. I’m one of the lucky ones who gets to do this almost daily, and I’ve often caught myself wondering – am I really reviewing every PR the right way?
Probably not always. So the real question is: what could I do better to make the process benefit the whole project, not just meet some short-term need? That’s why today I want to talk about the full PR spectrum: from the overly detailed, filled-with-comments kind, to the ones nobody likes, to the “just-ship-it” type that only exists to push code into production.
The “right here, right now” PR
This one’s rare, but when it happens, you need to make a quick decision – yes or no. I’ve seen these plenty of times, and usually they’re followed by a second and a third PR to fix the first one. Example: a button on the homepage is broken, my colleague rushes a fix, and because it’s urgent, I approve it. The code goes live, the bug resurfaces, we patch it again, and again deploy. Not great.
The oversized PR
You open it and immediately know you’re in trouble. You skim a few files – looks fine. Scroll further, lose track, postpone it for later. By the time you return, you’ve forgotten where you left off. You jump to the end, skim, get annoyed, and either approve without full review or ignore it altogether.
The PR that shouldn’t exist
The worst kind. They pile up for various reasons: sometimes feature-related, sometimes because everything gets merged on the same day. Anyone joining a new project has probably heard: “This PR isn’t reviewable, it’s been open for a month, we just keep merging other branches into it.”
I’ve never understood how anyone can ship that much code to production at once. Sure, every change might be tested, every commit reviewed, but wouldn’t it be easier to use feature flags? Once in prod, the risk of breakage skyrockets – and “fail-fast” feels like the safer road.
Where’s the description?
Plenty of PRs give you zero context: What’s being changed? Why? Is it a bugfix or a feature? Does it break existing functionality? Was it tested? This info should be in the PR. If it’s not, ask – or just reject it. Tools like PR templates exist for a reason.
Wait, do we even need PRs?
Some argue we could skip them entirely. That mindset often comes from pain: careless feedback given to an introvert, last-minute sprint crunch, or overloaded reviews on top of deadlines. The issue isn’t that reviews are “bad” – it’s that the process itself needs fixing.
The follow-through problem
No matter how big or small, a PR needs review. We all promise to check them daily, but in reality, we slip. Teams bring it up in retrospectives, improve for a week or two, and then fall back into the same cycle.
I even read about one tactic: if you don’t review a PR within 24h, a bot shames you in a Slack #board_of_shame channel. Personally, I think public flogging is a terrible motivator, but hey – whatever works. If you know better methods, share them.
Style is the distraction
We often overemphasize style. Different style ≠ bad code. If we focus only on commas and brackets, we lose sight of the real goal: shipping bug-free code. Style issues can be solved with tools. More important is understanding what the code actually does. If someone outside the project reviews just for style, that’s wasted effort.
So what does a good PR look like?
Criticism is easy, but what should we aim for? A decent PR should be:
-
Clearly described – short, precise title, link to the task, all changes explained.
-
Not too big – if the diff fits into four A4 pages, that’s manageable.
-
Style checked automatically, not manually nitpicked.
-
Reviewed and approved quickly.
-
Containing no more than 3 commits (or squashed before merging).
That’s the theory. But in practice – how often have you seen such PRs?
