7 min read • Guide 679 of 877
Improving Code Review Quality
High-quality code reviews catch bugs, share knowledge, and improve code quality across the team. GitScrum helps track review status, identify review bottlenecks, and maintain visibility into the review process as part of the development workflow.
Review Quality Framework
What to Review
CODE REVIEW FOCUS AREAS:
┌─────────────────────────────────────────────────────────────┐
│ │
│ HIGH PRIORITY (Always Review): │
│ ✓ Logic correctness │
│ ✓ Security vulnerabilities │
│ ✓ Error handling │
│ ✓ Edge cases │
│ ✓ Breaking changes │
│ │
│ MEDIUM PRIORITY (Usually Review): │
│ ✓ Performance implications │
│ ✓ Test coverage │
│ ✓ API design │
│ ✓ Code maintainability │
│ ✓ Documentation accuracy │
│ │
│ LOW PRIORITY (Automate When Possible): │
│ ○ Formatting and style │
│ ○ Import ordering │
│ ○ Naming conventions │
│ ○ Simple linting rules │
│ │
│ AUTOMATE: Use linters, formatters, type checkers │
│ Human reviewers focus on what machines can't do │
└─────────────────────────────────────────────────────────────┘
Review Checklist
REVIEWER CHECKLIST:
┌─────────────────────────────────────────────────────────────┐
│ │
│ BEFORE REVIEWING: │
│ ☐ Read PR description and linked task │
│ ☐ Understand the context and goal │
│ ☐ Check CI status (tests pass?) │
│ │
│ CORRECTNESS: │
│ ☐ Does this solve the stated problem? │
│ ☐ Are there logic errors? │
│ ☐ Are edge cases handled? │
│ ☐ Is error handling appropriate? │
│ │
│ SECURITY: │
│ ☐ Input validation present? │
│ ☐ No sensitive data exposed? │
│ ☐ Authentication/authorization correct? │
│ ☐ No SQL injection, XSS, etc.? │
│ │
│ QUALITY: │
│ ☐ Code is readable and maintainable? │
│ ☐ Tests are adequate? │
│ ☐ No obvious performance issues? │
│ ☐ Follows team patterns? │
│ │
│ AFTER REVIEWING: │
│ ☐ Feedback is constructive? │
│ ☐ Blocking vs non-blocking clear? │
│ ☐ Questions asked when unclear? │
└─────────────────────────────────────────────────────────────┘
Feedback Quality
Writing Good Comments
FEEDBACK QUALITY GUIDE:
┌─────────────────────────────────────────────────────────────┐
│ │
│ BAD COMMENT: │
│ "This is wrong." │
│ │
│ BETTER COMMENT: │
│ "This will fail when `user` is null. Consider adding │
│ a null check on line 45." │
│ │
│ ───────────────────────────────────────────────── │
│ │
│ BAD COMMENT: │
│ "Use a better name." │
│ │
│ BETTER COMMENT: │
│ "The variable `d` is unclear. Maybe `daysSinceLogin` │
│ would convey the purpose better?" │
│ │
│ ───────────────────────────────────────────────── │
│ │
│ BAD COMMENT: │
│ "This is inefficient." │
│ │
│ BETTER COMMENT: │
│ "This approach is O(n²) due to nested loops. For large │
│ datasets, consider using a Map for O(n) lookup. │
│ Happy to discuss if this complexity is needed here." │
│ │
│ ───────────────────────────────────────────────── │
│ │
│ COMMENT PREFIX CONVENTIONS: │
│ [blocking]: Must fix before merge │
│ [nit]: Minor, non-blocking suggestion │
│ [question]: Need clarification │
│ [praise]: Great work, keep doing this │
└─────────────────────────────────────────────────────────────┘
Constructive Tone
TONE GUIDELINES:
┌─────────────────────────────────────────────────────────────┐
│ │
│ USE "WE" INSTEAD OF "YOU": │
│ ✗ "You should use early return here" │
│ ✓ "We prefer early returns in this codebase" │
│ │
│ ASK QUESTIONS INSTEAD OF COMMANDING: │
│ ✗ "Change this to use async/await" │
│ ✓ "What do you think about using async/await here?" │
│ │
│ EXPLAIN THE WHY: │
│ ✗ "Add error handling" │
│ ✓ "Adding error handling here would prevent silent │
│ failures that could corrupt user data." │
│ │
│ ACKNOWLEDGE GOOD WORK: │
│ ✗ [silence on good code] │
│ ✓ "Nice use of the factory pattern here. Clean solution." │
│ │
│ BE SPECIFIC: │
│ ✗ "The architecture is wrong" │
│ ✓ "This creates a circular dependency between modules. │
│ Consider extracting the shared logic to a third module."│
│ │
│ REMEMBER: Code review is teaching, not judging │
└─────────────────────────────────────────────────────────────┘
Review Efficiency
PR Size and Scope
OPTIMAL PR CHARACTERISTICS:
┌─────────────────────────────────────────────────────────────┐
│ │
│ SIZE GUIDELINES: │
│ │
│ Lines │ Review Quality │ Time │ Recommendation │
│─────────┼────────────────┼─────────┼──────────────────────│
│ <100 │ Excellent │ 15 min │ ✅ Ideal │
│ 100-400 │ Good │ 30 min │ ✅ Acceptable │
│ 400-800 │ Declining │ 60 min │ ⚠️ Split if possible│
│ 800+ │ Poor │ 90+ min │ ❌ Must split │
│ │
│ SCOPE GUIDELINES: │
│ │
│ ✅ GOOD PR SCOPE: │
│ • Single feature or bug fix │
│ • One logical change │
│ • Includes related tests │
│ • Self-contained │
│ │
│ ❌ BAD PR SCOPE: │
│ • Multiple unrelated changes │
│ • "While I was here" refactoring │
│ • Feature + unrelated cleanup │
│ • Large reformatting + logic changes │
│ │
│ RULE: If you can't describe the PR in one sentence, │
│ it probably should be multiple PRs. │
└─────────────────────────────────────────────────────────────┘
Review Prioritization
REVIEW PRIORITY MATRIX:
┌─────────────────────────────────────────────────────────────┐
│ │
│ REVIEW ORDER: │
│ │
│ 1. BLOCKING OTHERS │
│ PRs that are blocking other team members │
│ → Review within 2 hours │
│ │
│ 2. SMALL AND URGENT │
│ Bug fixes, hotfixes, time-sensitive │
│ → Review within 4 hours │
│ │
│ 3. AWAITING SECOND REVIEW │
│ One approval, needs final sign-off │
│ → Review within 4 hours │
│ │
│ 4. STANDARD │
│ Normal feature work │
│ → Review within 24 hours │
│ │
│ 5. LARGE / COMPLEX │
│ Needs dedicated time │
│ → Schedule review session │
│ │
│ DAILY HABIT: │
│ Check for pending reviews at start and end of day │
│ "Review first, code second" mentality │
└─────────────────────────────────────────────────────────────┘
Metrics and Improvement
Review Metrics
CODE REVIEW METRICS:
┌─────────────────────────────────────────────────────────────┐
│ │
│ TIME TO FIRST REVIEW: │
│ From PR creation to first reviewer comment │
│ Target: <4 hours | Current: 6 hours │
│ │
│ REVIEW CYCLE TIME: │
│ From PR creation to final approval │
│ Target: <24 hours | Current: 32 hours │
│ │
│ REVIEW ITERATIONS: │
│ Number of request changes rounds │
│ Target: ≤2 | Current: 2.3 │
│ │
│ COMMENTS PER PR: │
│ Average constructive comments │
│ Target: 3-10 | Current: 5.2 │
│ │
│ DEFECTS FOUND IN REVIEW: │
│ Bugs caught before merge │
│ Track: Severity of issues found │
│ │
│ REVIEW COVERAGE: │
│ % of code that gets reviewed │
│ Target: 100% | Current: 100% │
└─────────────────────────────────────────────────────────────┘