Code Review in Practice: From Checklist to Engineering Culture
Code Review is not "code inspection" — it is "code communication." This article covers a practical Review Checklist, how to write effective Review comments, and how to design a Review process that improves team capability — for teams adopting or optimizing their Code Review workflow.
The Bottom Line: Code Review Is an Investment, Not a Cost
Many teams treat Code Review as a “gate before deployment” — CI passes, Review approves, merge and ship. But the real value of Review is not “catching bad code” — it is “improving everyone’s code quality through the review process.”
This article covers three layers: the Review Checklist, the art of Review comments, and Review process design.
1. The Review Checklist: What to Look At, What to Skip
Priority
| Priority | Item | Note |
|---|---|---|
| P0 | Logic correctness | Edge cases, error paths, concurrency safety |
| P0 | Security | SQL injection, XSS, permission checks, secrets |
| P1 | Maintainability | Naming, function length, duplication, comments |
| P1 | Test coverage | Tests for new code, edge case coverage |
| P2 | Performance | Unnecessary loops, N+1 queries, memory leaks |
| P3 | Code style | Formatting, indentation — leave to tools |
Do not spend time on P3. Code style is a formatter job (Prettier, ESLint, Rustfmt), not a human job. If you find style issues in Review, add a formatting check to CI instead.
The Golden Range
Keep PRs under 400 lines. Beyond 400 lines, bug detection rate drops sharply and reviewer fatigue spikes.
2. The Art of Review Comments
Four Tones
| Type | Example | Effect |
|---|---|---|
| Command | ”Change to Map” | Resistance |
| Question | ”Why use Map instead of Object?” | Discussion |
| Suggestion | ”How about trying Map here?” | Openness |
| Educational | ”Map guarantees key order, which helps here” | Learning |
Principles
Start with positive feedback. Say at least “this design is clean” or “great naming” in every review round.
Use “we” not “you.” “Are we missing this edge case?” vs “You missed this edge case.”
Label blocking vs non-blocking. Mark each comment as “must fix” (blocking) or “suggestion” (nit). Without labels, the author does not know what is mandatory.
3. Review Process Design
SLA
| PR Size | Response SLA | Note |
|---|---|---|
| ≤ 100 lines | Within 2 hours | Small changes, quick review |
| 100-400 lines | Within 4 hours | Normal PR, same day |
| > 400 lines | Split PR | Large PRs should be reviewed in person |
Two-Round Rule
Round 1: Reviewer gives feedback, author revises. Round 2: Reviewer confirms changes. Beyond two rounds, communication is broken — switch to real-time discussion.
Toolchain
| Purpose | Tool |
|---|---|
| Formatting | Prettier / ESLint (CI auto-check) |
| Static analysis | SonarQube / CodeQL (CI auto-check) |
| Review platform | GitHub Pull Request / GitLab Merge Request |
| Review template | PR template with auto-loaded Checklist |
Summary
| Layer | Key Principle | Common Mistake |
|---|---|---|
| Scope | Logic + security first, style to tools | Spending time on formatting |
| Tone | Questions over commands, positive first | Only criticism |
| PR size | ≤ 400 lines | One-shot large PRs |
| Rounds | Two-round rule, then real-time | Ping-pong back and forth |
| SLA | Respond within 4 hours | Reviewing just before deployment |
A good Code Review is not measured by how many bugs it caught — it is measured by how much the team improved through it. The best review comment is one where the author thinks “I never thought of that” — not “okay, I’ll change it.”
Need engineering practice consulting or team workflow optimization? Contact us — tell us your current situation and pain points, feasibility within 24 hours.
Related reading
- How to Run a Technical Review — upstream technical decision-making process for Code Review
- Ops Automation Script Patterns — shared engineering and maintainability principles
FAQ
What should I look for in a Code Review?
Priority order: ① Logic correctness — edge cases, error paths, concurrency safety; ② Security — SQL injection, XSS, permission bypass, sensitive data exposure; ③ Maintainability — naming clarity, function length, duplication, comments; ④ Test coverage — tests for new code, edge case coverage; ⑤ Performance — unnecessary loops, N+1 queries, memory leaks. Do not spend time on code style — that is the formatter job.
How do I write review comments that do not offend?
Use questions instead of commands. "Why use Map instead of Object here?" is better than "Change to Map." Use "we" instead of "you." "Are we missing this edge case?" is better than "You missed this edge case." Start with positive feedback before suggestions. "The design is clear; one small suggestion…" is better than jumping straight to criticism.
What if reviews are too slow?
Check if PRs are too large. Bug detection rate drops sharply beyond 400 lines. Recommendations: ① Limit PR size (≤400 lines or ≤10 files); ② Set SLA (e.g., start review within 4 hours); ③ Small PRs async, large PRs face-to-face; ④ If the reviewer has no time, the author can walk through the change verbally — the reviewer only needs to nod or shake their head.
What if the team has no time for Code Review?
No time for review means review time was not budgeted. Solutions: ① Include review time in Sprint estimates — coding time includes reviewing others code; ② Set review SLA (respond within 4 hours), PRs exceeding SLA can be merged with an "unreviewed" label; ③ Assign dedicated reviewers instead of full-team review to reduce coordination overhead.
This article comes from AI Enable Harness front-line delivery practice. Need a similar system or optimization service?
Subscribe to Updates
Get notified when new articles are published. No spam, occasional updates only.
Subscribe →