Should Code Smells Block a Pull Request?

I've seen it a hundred times: a reviewer blocks a PR because a method is too long. Or because there's some duplicated code. And the whole team grinds to a halt. But here's the thing—code smells are just hints, not hard stops. Unless they're pointing to something that'll actually bite you, blocking on them is usually a waste of time.

Google's own guidance says the goal of review is to improve code health, not to achieve perfection. They even say 'there is no such thing as perfect code, there is only better code.' So if a change makes things better overall, approve it. Don't get hung up on every little smell.

So what's a code smell, anyway?

It's a surface clue that something might be off deeper down. SonarQube calls them maintainability issues—stuff like long methods, deep nesting, duplicated logic, or poor names. But a smell doesn't break anything by itself. It's a nudge to look closer. The real question isn't 'is there a smell?' but 'does this smell make the change risky?'

Why most smells shouldn't block a merge

Take cyclomatic complexity. It's just 1 plus the number of decision points. A method with 14 if-statements has a complexity of 15. But if it's a well-tested parser, who cares? Blocking a 200-line change over that is silly. Instead, ask: can we simplify this without breaking anything? If not, approve and move on.

I've been on teams where reviewers nitpick naming and duplication. The author spends days refactoring. Meanwhile, the bug fix sits unmerged. And guess what? Your DORA metrics tank. Change lead time goes up, deployment frequency goes down. Nobody wants that.

When a smell should block

Some smells are red flags. Duplicated security checks? That's a problem. If you copy-paste validation from another endpoint, you might miss a critical fix. OWASP's Top 10 for 2025 puts Broken Access Control at #1, found in 3.73% of apps tested. So if a new access control path duplicates logic instead of reusing a vetted helper, block it. Same goes for deeply nested functions handling user input—could hide an injection flaw.

Here's a real example: a 300-line PR adds a new API endpoint. There's a 120-line method with nested conditionals (complexity ~25) and some copy-pasted validation. The author says it works. Should you block? Yes—but not because of the complexity. Block because the duplicated validation is a security risk. If the original endpoint had a path traversal fix that's missing here, you've just introduced a vulnerability.

How to triage smells without slowing down

Separate mechanical smells from semantic ones. Mechanical stuff—formatting, naming, import order—should be caught by linters and formatters before human review. ESLint can enforce style rules automatically. SonarQube's quality gate can block merges if new code coverage drops below 80% or duplication exceeds 3%. Let the robots handle that.

Humans should focus on semantic smells: duplicated business logic, leaky abstractions, missing edge cases. Those need judgment. When you spot one, label it clearly. Use labels like 'nit', 'suggestion', or 'blocking'. A nit about a long variable name? Fine to leave unresolved. A blocking comment about a missing security check? Not fine.

• Blocking: duplicated security checks, missing input validation, hardcoded credentials, untested error paths.
• Non-blocking: long methods, minor duplication, naming, missing comments, style inconsistencies.

One more thing: track how often smells actually cause problems. If a smell leads to a bug once every six months, it's probably not worth blocking every PR. If it causes an incident weekly, then yeah, block it.

Balancing speed and code health

Google's data shows small changes get faster feedback—median time to first response under an hour. That only works if reviewers don't nitpick. Their study found 97% of developers were satisfied with the review tool, and the top expectations were education and maintaining norms, not finding bugs. If you block on every smell, you kill those benefits.

But you can't ignore smells entirely. Technical debt ratio above 50% gets an E rating in SonarQube—meaning the codebase is a nightmare. The goal is to keep that ratio low over time by fixing smells incrementally, not by blocking every PR. Try 'LGTM with comments': approve while leaving unresolved comments for minor smells. Keeps velocity high and records the issue for later.

What I'd actually do

Set up a SonarQube quality gate that fails on new vulnerabilities, new security hotspots, and coverage below 80%—but not on code smells. Configure your linter to auto-fix style. In human review, block only on three things: security flaws, broken tests, and duplicated logic that could diverge dangerously. For everything else, leave a comment labeled 'nit' or 'suggestion' and approve. If a smell is severe enough to warrant a refactor, file a separate issue and let the author address it in a follow-up change. That's how you keep code health improving without turning review into a bottleneck.

And remember: perfect is the enemy of done. Ship it.

Sources

• Google Engineering Practices - Standard of Code Review - https://google.github.io/eng-practices/review/reviewer/standard.html
• SonarQube Server Docs - Understanding quality gates - https://docs.sonarsource.com/sonarqube-server/2026.1/quality-standards-administration/managing-quality-gates/introduction-to-quality-gates
• Modern Code Review: A Case Study at Google (ICSE-SEIP '18) - https://www.papercache.org/papers/mlsys/system/2026/03/25/modern-code-review-a-case-study-at-google
• OWASP Top Ten 2025 - https://owasp.org/Top10/2025/0x00_2025-Introduction/
• DORA - Software delivery performance metrics - https://dora.dev/guides/dora-metrics-four-keys/