06-22-2026, 12:36 PM
Code review quality varies enormously. A rubber-stamp "LGTM" on every PR is useless; nitpicking whitespace for three rounds is demoralising. Here's what actually matters.
Things worth flagging:
Correctness - will this actually work? Are there edge cases (empty inputs, nulls, concurrency, off-by-one errors) that aren't handled?
Security - is user input validated and sanitised? Are there SQL injection, XSS, or path traversal risks? Are secrets handled correctly (not logged, not in responses)?
Logic errors in error handling - does the code handle failure gracefully? Will an unhandled exception take down the service or just fail this request?
Naming and clarity - if you have to read a function twice to understand what it does, the name or structure needs work. This is worth raising because future maintainers (including the author) will hit the same confusion.
Missing tests - if this adds new behaviour, are there tests? If it fixes a bug, is there a regression test?
Things not worth bikeshedding:
How to give useful feedback:
Timebox it - a focused 30-minute review is more valuable than two hours of exhaustive comments that overwhelm the author.
Things worth flagging:
Correctness - will this actually work? Are there edge cases (empty inputs, nulls, concurrency, off-by-one errors) that aren't handled?
Security - is user input validated and sanitised? Are there SQL injection, XSS, or path traversal risks? Are secrets handled correctly (not logged, not in responses)?
Logic errors in error handling - does the code handle failure gracefully? Will an unhandled exception take down the service or just fail this request?
Naming and clarity - if you have to read a function twice to understand what it does, the name or structure needs work. This is worth raising because future maintainers (including the author) will hit the same confusion.
Missing tests - if this adds new behaviour, are there tests? If it fixes a bug, is there a regression test?
Things not worth bikeshedding:
- Formatting, if there's a linter enforcing it
- Minor style preferences that don't affect readability
- "I would have done it differently" without a concrete reason
How to give useful feedback:
- Be specific - "this could be clearer" is less useful than "the variable name d doesn't tell me what this stores"
- Explain why, not just what
- Distinguish blockers from suggestions - prefix with Blocker: / Suggestion: / Nit:
- Praise good work - it's not all criticism
Timebox it - a focused 30-minute review is more valuable than two hours of exhaustive comments that overwhelm the author.
