13 min read • Guide 35 of 877
Setting Up Effective Code Review Workflows
Code reviews are critical for maintaining quality, sharing knowledge, and catching bugs before production. But poorly designed review processes can become bottlenecks that frustrate developers and slow delivery. GitScrum's integrations and workflow features help establish review practices that are thorough yet efficient, ensuring quality without sacrificing velocity.
The Code Review Challenge
Common review problems:
| Problem | Impact |
|---|---|
| Reviews take too long | PRs pile up, context lost |
| Inconsistent standards | Quality varies by reviewer |
| Unclear expectations | Nitpicking vs. important feedback |
| Reviewer bottlenecks | One person blocks all PRs |
| No accountability | Reviews ignored or delayed |
GitScrum Integration Setup
Connecting Git Providers
Git Provider Integration:
SUPPORTED PLATFORMS:
├── GitHub (Cloud & Enterprise)
├── GitLab (Cloud & Self-hosted)
└── Bitbucket (Cloud & Server)
CONNECTION STEPS:
1. Settings → Integrations → Git Providers
2. Select provider
3. Authorize OAuth connection
4. Choose repositories to connect
5. Configure sync settings
SYNC OPTIONS:
┌─────────────────────────────────────────────────────────────┐
│ Git Integration Settings │
├─────────────────────────────────────────────────────────────┤
│ Repository: acme/frontend │
│ │
│ Sync Settings: │
│ ☑ Link branches to tasks (pattern: TASK-###) │
│ ☑ Link PRs to tasks │
│ ☑ Update task status on PR events │
│ ☑ Show PR status in task details │
│ ☑ Import PR comments as task comments │
│ │
│ Status Mapping: │
│ PR Opened → Move task to: [In Review ▼] │
│ PR Approved → Move task to: [Approved ▼] │
│ PR Merged → Move task to: [Done ▼] │
│ PR Closed → Move task to: [No change ▼] │
└─────────────────────────────────────────────────────────────┘
Automatic Task Linking
Branch Naming Convention:
PATTERN: [type]/TASK-[id]-[description]
Examples:
├── feature/TASK-456-user-search
├── fix/TASK-789-login-bug
├── refactor/TASK-101-cleanup-auth
└── chore/TASK-202-update-deps
Auto-Detection Flow:
┌─────────────────────────────────────────────────────────────┐
│ 1. Developer creates branch: │
│ git checkout -b feature/TASK-456-user-search │
│ │
│ 2. GitScrum detects pattern "TASK-456" │
│ │
│ 3. Task TASK-456 auto-updated: │
│ ├── Status: → In Progress │
│ ├── Branch: feature/TASK-456-user-search │
│ └── Activity: "Branch created by @alice" │
│ │
│ 4. When PR opened: │
│ ├── PR linked to task │
│ ├── Status: → In Review │
│ └── Activity: "PR #123 opened" │
│ │
│ 5. When PR merged: │
│ ├── Status: → Done │
│ └── Activity: "PR #123 merged to main" │
└─────────────────────────────────────────────────────────────┘
Reviewer Assignment
Automated Assignment Rules
Reviewer Assignment Configuration:
ASSIGNMENT STRATEGIES:
┌─────────────────────────────────────────────────────────────┐
│ Reviewer Assignment Rules │
├─────────────────────────────────────────────────────────────┤
│ │
│ Strategy: [Round Robin ▼] │
│ │
│ Options: │
│ ├── Round Robin: Rotate through team members │
│ ├── Load Balanced: Assign to person with fewest reviews │
│ ├── Code Ownership: Based on file paths changed │
│ ├── Random: Random selection from pool │
│ └── Manual: Author selects, with suggestions │
│ │
│ Reviewer Pool: [Frontend Team ▼] │
│ ├── @alice (senior) │
│ ├── @bob (senior) │
│ ├── @carol (mid) │
│ └── @david (mid) │
│ │
│ Required Reviewers: [2] │
│ │
│ Rules: │
│ ☑ At least 1 senior reviewer │
│ ☑ Cannot review own PRs │
│ ☑ Exclude on PTO (sync from calendar) │
│ □ Require specific reviewer for [security/*] │
└─────────────────────────────────────────────────────────────┘
Code Ownership Mapping
CODEOWNERS-style Assignment:
File Path Rules:
┌─────────────────────────────────────────────────────────────┐
│ Code Ownership Configuration │
├─────────────────────────────────────────────────────────────┤
│ │
│ Path Pattern Owners Required │
│ ───────────────────────────────────────────────────────────│
│ /src/api/* @api-team 1 │
│ /src/components/ui/* @ui-team 1 │
│ /src/auth/* @security-team 2 │
│ /src/payments/* @payments-lead 1 + @security │
│ /database/* @dba-team 1 │
│ *.sql @dba-team 1 │
│ /tests/* @qa-lead Optional │
│ /docs/* @tech-writer Optional │
│ * @default-reviewers 1 │
│ │
│ Priority: First match wins (top to bottom) │
└─────────────────────────────────────────────────────────────┘
Example PR:
Files changed:
├── src/auth/login.ts
├── src/auth/session.ts
└── src/components/LoginForm.tsx
Auto-assigned:
├── @security-team (2 required - auth files)
└── @ui-team (1 required - component file)
Review Load Balancing
Load-Balanced Assignment:
CURRENT REVIEW LOAD:
┌─────────────────────────────────────────────────────────────┐
│ REVIEWER WORKLOAD [This Week] │
├─────────────────────────────────────────────────────────────┤
│ │
│ Active Reviews: │
│ Alice: [██░░░░░░] 2 PRs (capacity: 4) ← Next assign │
│ Bob: [████░░░░] 4 PRs (capacity: 4) Full │
│ Carol: [███░░░░░] 3 PRs (capacity: 4) │
│ David: [██░░░░░░] 2 PRs (capacity: 4) ← Next assign │
│ │
│ Review Stats This Week: │
│ ├── Alice: 8 completed, avg 4h response │
│ ├── Bob: 6 completed, avg 6h response │
│ ├── Carol: 7 completed, avg 3h response │
│ └── David: 5 completed, avg 8h response │
│ │
│ Assignment Logic: │
│ 1. Filter: Not at capacity │
│ 2. Filter: Not on PTO │
│ 3. Filter: Not PR author │
│ 4. Sort by: Current active reviews (ascending) │
│ 5. Assign to: First in list │
└─────────────────────────────────────────────────────────────┘
Review SLAs and Tracking
Time-Based Expectations
Review SLA Configuration:
┌─────────────────────────────────────────────────────────────┐
│ Code Review SLAs │
├─────────────────────────────────────────────────────────────┤
│ │
│ First Response SLA: │
│ ├── P0/Critical: 2 hours │
│ ├── P1/High: 4 hours │
│ ├── P2/Normal: 8 hours (1 business day) │
│ └── P3/Low: 24 hours │
│ │
│ Review Completion SLA: │
│ ├── Small PR (<100 lines): 4 hours after first response │
│ ├── Medium PR (100-500 lines): 8 hours │
│ └── Large PR (>500 lines): 24 hours │
│ │
│ Escalation Rules: │
│ ├── 50% of SLA: Reminder to reviewer │
│ ├── 100% of SLA: Notify team lead │
│ ├── 150% of SLA: Escalate to eng manager │
│ └── 200% of SLA: Allow merge without full review │
│ │
│ Working Hours: Mon-Fri 9AM-6PM (team timezone) │
└─────────────────────────────────────────────────────────────┘
Review Status Dashboard
Review Pipeline Dashboard:
┌─────────────────────────────────────────────────────────────┐
│ CODE REVIEW PIPELINE [Last 7 Days ▼] │
├─────────────────────────────────────────────────────────────┤
│ │
│ Current Status: │
│ ├── Awaiting Review: 8 PRs │
│ ├── In Review: 5 PRs │
│ ├── Changes Requested: 3 PRs │
│ └── Approved (pending merge): 2 PRs │
│ │
│ SLA Performance: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ First Response ││
│ │ On Time: ████████████████████ 85% ││
│ │ Late: ████ 15% ││
│ │ ││
│ │ Review Completion ││
│ │ On Time: ██████████████████ 78% ││
│ │ Late: ██████ 22% ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ Aging PRs (SLA Risk): │
│ ├── 🔴 PR #456: 6h overdue (assigned: @bob) │
│ ├── 🟡 PR #461: 2h remaining (assigned: @carol) │
│ └── 🟡 PR #463: 3h remaining (assigned: @alice) │
│ │
│ Weekly Metrics: │
│ ├── PRs reviewed: 47 │
│ ├── Avg time to first response: 3.2h │
│ ├── Avg time to approval: 8.4h │
│ ├── Review cycles per PR: 1.4 │
│ └── Approval rate: 94% │
└─────────────────────────────────────────────────────────────┘
Review Checklists
Standardized Review Criteria
Code Review Checklist Template:
┌─────────────────────────────────────────────────────────────┐
│ PR REVIEW CHECKLIST │
├─────────────────────────────────────────────────────────────┤
│ │
│ FUNCTIONALITY │
│ □ Code does what the task description says │
│ □ Edge cases are handled │
│ □ Error handling is appropriate │
│ □ No obvious bugs │
│ │
│ CODE QUALITY │
│ □ Code is readable and self-documenting │
│ □ Functions/methods are appropriately sized │
│ □ No code duplication │
│ □ Follows project conventions and patterns │
│ │
│ TESTING │
│ □ Unit tests cover new functionality │
│ □ Tests are meaningful (not just for coverage) │
│ □ Edge cases are tested │
│ □ Tests pass locally and in CI │
│ │
│ SECURITY │
│ □ No sensitive data exposed │
│ □ Input validation in place │
│ □ Authentication/authorization correct │
│ □ No SQL injection or XSS vulnerabilities │
│ │
│ PERFORMANCE │
│ □ No obvious performance issues │
│ □ Database queries are efficient (no N+1) │
│ □ Appropriate caching considered │
│ │
│ DOCUMENTATION │
│ □ Complex logic has comments │
│ □ API changes are documented │
│ □ README updated if needed │
└─────────────────────────────────────────────────────────────┘
Checklist Enforcement:
├── All items must be checked before approval
├── Unchecked items require comment explaining why N/A
└── Automated check blocks merge if checklist incomplete
Context-Specific Checklists
Conditional Checklists:
TRIGGER: PR touches /src/api/*
CHECKLIST: API Review
┌─────────────────────────────────────────────────────────────┐
│ □ API versioning considered │
│ □ Request/response schemas validated │
│ □ Rate limiting in place │
│ □ Error responses follow standards │
│ □ OpenAPI spec updated │
│ □ Integration tests added │
└─────────────────────────────────────────────────────────────┘
TRIGGER: PR touches /database/* or *.sql
CHECKLIST: Database Review
┌─────────────────────────────────────────────────────────────┐
│ □ Migration is reversible │
│ □ Indexes added for new queries │
│ □ No breaking changes to existing columns │
│ □ Performance impact assessed │
│ □ Backup/rollback plan documented │
└─────────────────────────────────────────────────────────────┘
TRIGGER: PR label = "security"
CHECKLIST: Security Review
┌─────────────────────────────────────────────────────────────┐
│ □ Security team has reviewed │
│ □ Penetration testing if applicable │
│ □ Secrets rotated if exposed │
│ □ Audit logging in place │
│ □ Compliance requirements met │
└─────────────────────────────────────────────────────────────┘
Feedback Quality
Comment Categories
Structured Feedback Types:
COMMENT PREFIXES:
┌─────────────────────────────────────────────────────────────┐
│ Prefix Meaning Blocking? │
├─────────────────────────────────────────────────────────────┤
│ [BLOCKER] Must fix before merge Yes │
│ [ISSUE] Should fix, important Yes (usually) │
│ [SUGGEST] Consider changing No │
│ [NIT] Minor/style preference No │
│ [QUESTION] Need clarification Depends on answer │
│ [PRAISE] Good job, nice code No │
│ [FYI] Information sharing No │
└─────────────────────────────────────────────────────────────┘
Examples:
[BLOCKER] This SQL query is vulnerable to injection.
Please use parameterized queries.
[ISSUE] This function is 200 lines. Consider breaking
into smaller functions for testability.
[SUGGEST] You could simplify this with a reduce():
const total = items.reduce((sum, i) => sum + i.value, 0);
[NIT] I'd prefer `isActive` over `active` for boolean naming.
[PRAISE] Really clean implementation of the cache layer!
[QUESTION] What happens if the user is null here?
Feedback Tracking
Review Comment Analytics:
COMMENT RESOLUTION:
┌─────────────────────────────────────────────────────────────┐
│ PR #456: Add user search │
├─────────────────────────────────────────────────────────────┤
│ Comments: 12 total │
│ │
│ By Type: │
│ ├── BLOCKER: 1 → Resolved ✓ │
│ ├── ISSUE: 3 → 2 Resolved, 1 Pending │
│ ├── SUGGEST: 4 → 2 Accepted, 1 Declined, 1 Pending │
│ ├── NIT: 2 → Both Resolved │
│ └── PRAISE: 2 → N/A │
│ │
│ Resolution Rate: 83% │
│ Pending Actions: 2 │
│ │
│ Can Merge: No (1 blocker pending) │
└─────────────────────────────────────────────────────────────┘
Team Review Patterns:
┌─────────────────────────────────────────────────────────────┐
│ REVIEW PATTERNS [Last 30 Days] │
├─────────────────────────────────────────────────────────────┤
│ │
│ Comments per PR (avg): │
│ ├── Alice: 8.2 (40% ISSUE, 30% SUGGEST, 20% NIT) │
│ ├── Bob: 4.1 (60% ISSUE, 20% BLOCKER, 20% PRAISE) │
│ ├── Carol: 6.5 (50% SUGGEST, 30% QUESTION, 20% NIT) │
│ └── David: 3.2 (70% ISSUE, 20% SUGGEST, 10% PRAISE) │
│ │
│ Most Common Issues Found: │
│ ├── Missing error handling (23%) │
│ ├── Insufficient tests (19%) │
│ ├── Code duplication (15%) │
│ ├── Performance concerns (12%) │
│ └── Documentation gaps (11%) │
└─────────────────────────────────────────────────────────────┘
Approval Workflows
Multi-Stage Approvals
Approval Requirements:
STANDARD PR:
┌─────────────────────────────────────────────────────────────┐
│ Requirements for Merge: │
│ ├── 1+ approval from team │
│ ├── All blockers resolved │
│ ├── CI checks passing │
│ └── No unresolved threads │
└─────────────────────────────────────────────────────────────┘
SENSITIVE AREAS:
┌─────────────────────────────────────────────────────────────┐
│ Path: /src/auth/*, /src/payments/* │
│ Requirements: │
│ ├── 2+ approvals │
│ ├── 1 must be from security team │
│ ├── All checklists complete │
│ └── Extended CI (security scans) │
└─────────────────────────────────────────────────────────────┘
INFRASTRUCTURE:
┌─────────────────────────────────────────────────────────────┐
│ Path: /infrastructure/*, *.terraform │
│ Requirements: │
│ ├── DevOps team approval │
│ ├── Cost estimate reviewed │
│ ├── Rollback plan documented │
│ └── Change window scheduled │
└─────────────────────────────────────────────────────────────┘
Protected Branch Rules
Branch Protection Integration:
main/master:
┌─────────────────────────────────────────────────────────────┐
│ Protection Rules: │
│ ├── ☑ Require pull request before merging │
│ │ ├── Required approvals: 2 │
│ │ ├── Dismiss stale approvals on new commits: Yes │
│ │ └── Require review from code owners: Yes │
│ ├── ☑ Require status checks to pass │
│ │ ├── CI: Build │
│ │ ├── CI: Unit Tests │
│ │ ├── CI: Integration Tests │
│ │ └── CI: Security Scan │
│ ├── ☑ Require conversation resolution │
│ ├── ☑ Require signed commits │
│ └── ☑ Include administrators │
└─────────────────────────────────────────────────────────────┘
develop:
┌─────────────────────────────────────────────────────────────┐
│ Protection Rules: │
│ ├── ☑ Require pull request before merging │
│ │ └── Required approvals: 1 │
│ └── ☑ Require status checks to pass │
│ └── CI: Build, Unit Tests │
└─────────────────────────────────────────────────────────────┘
Best Practices
For Authors
- Keep PRs small — <300 lines ideal, <500 max
- Write good descriptions — Context, what, why, how
- Self-review first — Catch obvious issues
- Respond promptly — Address feedback quickly
- Don't take it personally — Reviews improve code, not judge people
For Reviewers
- Be timely — Respect the SLA
- Be specific — Point to exact lines, suggest fixes
- Be kind — Critique code, not people
- Prioritize feedback — Blockers > Issues > Nits
- Learn while reviewing — It's knowledge sharing
For Teams
- Define clear standards — Document what good looks like
- Rotate reviewers — Spread knowledge
- Track metrics — Improve over time
- Celebrate good reviews — Reinforce quality
- Regular retrospectives — Refine the process