Code Review Best Practices

Code review is the highest-leverage quality practice in software engineering. Studies show it catches 60-90% of defects before they reach production. But poorly run reviews — rubber-stamp approvals, nitpick wars, week-long review queues — destroy both quality and developer morale. This guide covers how to make code review fast, valuable, and sustainable.

---

What Reviewers Should Focus On

Priority	What to Check	Examples
1. Design	Does the approach make sense?	Correct abstraction, right pattern for the problem
2. Correctness	Does it work correctly?	Edge cases, error handling, concurrent access
3. Security	Any vulnerabilities?	Input validation, auth checks, data exposure
4. Readability	Can someone else understand this in 6 months?	Clear naming, appropriate comments, logical structure
5. Testing	Are tests meaningful?	Edge cases tested, not just happy path

What NOT to focus on (automate these instead):

• Formatting and style → prettier, black, gofmt
• Import ordering → isort, eslint
• Type safety → TypeScript, mypy
• Known anti-patterns → semgrep, eslint rules

---

Review Process

Author                           Reviewer
  │                                 │
  ├── Self-review before PR         │
  │   (catch obvious issues)        │
  │                                 │
  ├── Write clear PR description    │
  │   - What changed and why        │
  │   - How to test                 │
  │   - Screenshots (if UI)         │
  │                                 │
  ├── Request review ──────────────▶│
  │                                 ├── Read PR description first
  │                                 ├── Understand the context
  │                                 ├── Review for design first
  │                                 ├── Then correctness
  │                                 ├── Leave constructive comments
  │◀── Address feedback ───────────├── Approve or request changes
  │                                 │
  ├── Merge (author merges)         │
  └── Deploy                        │

---

Review Speed

Metric	Target	Why
First response time	< 4 business hours	Don’t block the author’s flow
Total review time	< 1 business day	Keep PRs moving, avoid stale context
PR size	< 400 lines changed	Smaller PRs get better reviews
Review rounds	≤ 2 rounds	After 2 rounds, pair and resolve live

---

Giving Constructive Feedback

Instead Of	Try
”This is wrong"	"I think this might cause X because Y. What if we Z instead?"
"Why did you do this?"	"I’m curious about the approach here — could you help me understand?"
"This is bad code"	"This could be simplified by extracting into a function”
Nitpicking style	Configure a linter to enforce style automatically

Comment Prefixes

Prefix	Meaning	Action Required?
nit:	Minor suggestion, non-blocking	No
suggestion:	Improvement idea	Discuss, not mandatory
question:	Seeking understanding	Clarify in comment
issue:	Must be addressed	Yes, blocking
praise:	Something done well	No (but important!)

---

Anti-Patterns

Anti-Pattern	Problem	Fix
Rubber stamping	”LGTM” without reading	Review checklist, minimum review time
Nitpick wars	Blocking PRs over formatting	Automate style enforcement
Gatekeeping	Only senior devs can approve	Trust juniors for non-critical code
Huge PRs	1000+ line PRs get poor reviews	Enforce PR size limits (400 lines)
Review queue rot	PRs sit for days unreviewed	Review SLA, rotation system
Ego reviews	”I would have done it differently”	If it works correctly, it’s fine

---

Checklist

• PR template with description, testing, screenshots
• Automated checks: lint, type check, tests, security scan
• Review SLA: first response < 4 business hours
• PR size limit: < 400 lines changed
• Comment conventions: nit, suggestion, issue, praise
• Self-review before requesting review
• Reviewer rotation to spread knowledge
• Pair review for 2+ round reviews (resolve live)

---

:::note[Source] This guide is derived from operational intelligence at Garnet Grid Consulting. For engineering practices consulting, visit garnetgrid.com. :::