Code Review
debt(d7/e5/b5/t5)
Closest to 'only careful code review or runtime testing' (d7). The irony: poor code review practices are themselves only detectable through meta-review or team retrospectives. While GitHub/GitLab can show PRs merged without review, and detection_hints mention 'review comments only about style not logic', no automated tool reliably catches rubber-stamping or superficial reviews. You need careful observation of team patterns to detect dysfunction.
Closest to 'touches multiple files / significant refactor in one component' (e5). Fixing poor code review culture requires changing team habits, updating PR templates, creating checklists, training reviewers, and potentially restructuring how work is broken into PRs. The quick_fix suggests a checklist approach, but implementing effective review practices across a team is more than a one-line change—it's a process overhaul affecting multiple workflows.
Closest to 'persistent productivity tax' (b5). Code review applies to all PHP contexts (web, cli, queue-worker) per applies_to. A dysfunctional review process becomes a persistent drag: either rubber-stamping lets bugs through constantly, or overly pedantic reviews slow every PR. The practice shapes team velocity and knowledge distribution over time, but doesn't quite 'define the system's shape' architecturally.
Closest to 'notable trap' (t5). The misconception field states clearly: 'Code review is primarily about catching bugs. Research shows reviews catch fewer bugs than automated testing — their real value is knowledge sharing, consistency enforcement, and collective code ownership.' This is a documented gotcha that most experienced developers eventually learn, but newcomers consistently overweight bug-catching and underweight the social/knowledge benefits.
Also Known As
TL;DR
Explanation
Code review is one of the most cost-effective quality practices: bugs caught at review are orders of magnitude cheaper to fix than in production. Effective reviews focus on correctness, security, design quality, and knowledge transfer — not style (automate that with linters). Reviewers should ask: Does this code do what it claims? Are edge cases handled? Are there security implications? Is it testable? Timely reviews (within hours, not days) and a respectful culture are essential. Pair programming can replace some async review in high-trust teams.
Common Misconception
Why It Matters
Common Mistakes
- Reviewing style and formatting instead of logic and design — automate style with linters, save human attention for intent.
- Approving large PRs without reading them carefully to avoid conflict — large PRs should be split, not rubber-stamped.
- Leaving vague comments ("fix this") without explanation or suggestion — reviewers should explain the problem and propose a direction.
- Treating code review as a gate rather than a conversation — the goal is shared understanding, not gatekeeping.
Avoid When
- Using code review as a gatekeeping ritual that blocks merges for trivial style issues — automate style with linters.
- Reviewing 1000-line PRs — large reviews are ineffective; keep PRs small and focused.
- Reviewing without running the code or tests — a review that only reads the diff misses runtime behaviour.
- Nitpicking in a way that demoralises authors — distinguish must-fix blocking issues from optional suggestions.
When To Use
- Every change to a shared codebase — a second pair of eyes catches bugs, design issues, and missing test cases.
- Enforcing architectural decisions and conventions that cannot be automated.
- Knowledge sharing — reviewers learn the codebase and authors learn from reviewer expertise.
- Security-sensitive changes — a dedicated security review catches vulnerabilities that functional review misses.
Code Examples
// Code review anti-patterns:
// Reviewer: 'LGTM' after 30 seconds on a 500-line PR
// No review checklist — misses security, performance, edge cases
// Author defensive about feedback — treats review as criticism
// PRs too large — impossible to review meaningfully
// No review culture — all PRs merged by author without review
// Better:
// PRs < 400 lines
// Checklist: correctness, security, tests, naming, edge cases
// Reviewer explains why, not just what to change# Security-focused code review checklist (PHP)
# Authentication & Authorisation
[ ] Session regenerated after login
[ ] Every endpoint has authorisation check
[ ] Sensitive actions require re-authentication
# Input Handling
[ ] All user input validated before use
[ ] No raw user input in SQL queries
[ ] HTML output escaped with htmlspecialchars()
[ ] File uploads: MIME check + random filename + outside webroot
# Cryptography
[ ] Passwords: password_hash()/password_verify() — not md5/sha1
[ ] Tokens: random_bytes() — not rand()/uniqid()
[ ] Sensitive comparisons: hash_equals()
# Error Handling
[ ] display_errors = Off in production
[ ] No stack traces in API responses
[ ] Errors logged internally, not exposed
# Dependencies
[ ] composer audit passes with no critical CVEs