Conducting Effective Code Reviews

Code reviews can be a bottleneck or a force multiplier. Effective reviews improve code quality, spread knowledge, and catch bugs early. Poor reviews create friction, delay work, and damage team morale. GitScrum integrates with GitHub/GitLab to make reviews visible and efficient.

Code Review Goals

Review Process

Submission Best Practices

PR AUTHOR CHECKLIST
═══════════════════

BEFORE SUBMITTING:
- [ ] Self-reviewed the diff
- [ ] Tests pass locally
- [ ] Linter passes
- [ ] No debug code left
- [ ] Description explains why

GOOD PR DESCRIPTION:
─────────────────────────────────────
## What
Brief description of the change.

## Why
Context and motivation.

## How
Technical approach taken.

## Testing
How this was verified.

## Screenshots (if UI)
Before/after if applicable.

## Related
Links to GitScrum task, docs, etc.
─────────────────────────────────────

PR Size Guidelines

PR SIZE RECOMMENDATIONS
═══════════════════════

IDEAL:     50-200 lines changed
ACCEPTABLE: 200-400 lines changed
TOO LARGE: 400+ lines changed

WHY SMALL PRs:
├── Easier to review thoroughly
├── Faster turnaround
├── Less risk per change
├── Simpler to revert
└── Better knowledge transfer

SPLITTING STRATEGIES:
├── Feature flags for partial work
├── Extract refactoring separately
├── Separate tests from implementation
├── Infrastructure vs logic
└── Backend vs frontend

Reviewer Process

REVIEW WORKFLOW
═══════════════

PHASE 1: UNDERSTAND (5 min)
├── Read PR description
├── Check linked task context
├── Understand the goal
└── Review test plan

PHASE 2: HIGH-LEVEL (10 min)
├── Is the approach right?
├── Any architectural concerns?
├── Missing components?
└── Correct scope?

PHASE 3: DETAILED (15-30 min)
├── Logic correctness
├── Edge cases handled
├── Error handling
├── Performance implications
├── Security considerations
└── Test coverage

PHASE 4: RESPOND (5 min)
├── Organize feedback
├── Prioritize comments
├── Approve, request changes, or comment
└── Be responsive to questions

Feedback Framework

Comment Types

COMMENT CLASSIFICATION
══════════════════════

BLOCKING (must fix):
├── Bugs and logic errors
├── Security vulnerabilities
├── Missing critical tests
├── Breaking changes
└── Violates core patterns

NON-BLOCKING (should consider):
├── Performance suggestions
├── Alternative approaches
├── Minor improvements
├── Style preferences (beyond linter)
└── Nice-to-haves

PRAISE (important!):
├── Clever solutions
├── Good test coverage
├── Clean refactoring
├── Good documentation
└── Learning moments

FORMAT:
─────────────────────────────────────
[blocking] This will fail if user is null

[nit] Consider renaming to `isValidUser`

[praise] Great handling of the edge case!
─────────────────────────────────────

Constructive Feedback

FEEDBACK EXAMPLES
═════════════════

INSTEAD OF:                     TRY:
───────────────────────────────────────────────────
"This is wrong"                 "This might cause X
                                 because Y. Consider Z."

"Why didn't you...?"            "Would it help to...?"

"This is confusing"             "I'm having trouble 
                                 following this. Could
                                 you add a comment or
                                 rename to clarify?"

"This is slow"                  "This will run N times.
                                 Consider memoizing."

"Don't do it this way"          "The team pattern for
                                 this is X. Here's an
                                 example: [link]"
───────────────────────────────────────────────────

What to Focus On

REVIEW PRIORITY
═══════════════

HIGH PRIORITY:
├── Correctness (does it work?)
├── Security (is it safe?)
├── Tests (is it verified?)
├── Edge cases (is it complete?)
└── Breaking changes (is it compatible?)

MEDIUM PRIORITY:
├── Performance (is it efficient?)
├── Readability (is it clear?)
├── Error handling (is it robust?)
├── Documentation (is it explained?)
└── Architecture (does it fit?)

LOW PRIORITY (often linter's job):
├── Formatting
├── Naming conventions
├── Import order
├── Line length
└── Trivial style choices

Review Culture

Team Norms

CODE REVIEW NORMS
═════════════════

TIMING:
├── Reviews happen within 4 hours
├── Interrupt work for reviews
├── Reviews before new work
└── No PRs over the weekend

PARTICIPATION:
├── Everyone reviews (not just seniors)
├── Rotate reviewers
├── Cross-team reviews for context
└── At least 2 reviewers for critical paths

ATTITUDE:
├── Assume good intent
├── Be humble (you might be wrong)
├── Separate ego from code
├── Thank reviewers
└── Learn from feedback

Reducing Friction

REDUCING REVIEW FRICTION
════════════════════════

AUTHOR CAN:
├── Keep PRs small
├── Write good descriptions
├── Self-review first
├── Respond quickly to feedback
└── Don't take it personally

REVIEWER CAN:
├── Review promptly
├── Be specific and actionable
├── Distinguish blocking vs nit
├── Offer solutions, not just problems
└── Approve when "good enough"

TEAM CAN:
├── Automate style checks
├── Document patterns
├── Review guidelines
├── Celebrate good reviews
└── Track review metrics

GitScrum Integration

Visibility

REVIEW VISIBILITY IN GITSCRUM
═════════════════════════════

TASK CARD SHOWS:
├── Linked PR(s)
├── Review status
├── CI status
├── Reviewers assigned
└── Time in review

BOARD COLUMN:
├── "In Review" column
├── Cards auto-move when PR opens
├── Auto-move on merge
└── Visible review queue

DASHBOARD:
├── Open PRs needing review
├── Average review time
├── Review load per person
└── Stale PRs flagged

Best Practices

For Effective Reviews

1. Review daily — Don't let PRs age
2. Smallest PRs win — Encourage splitting
3. Automate the obvious — Linters, formatters
4. Be a mentor — Teach, don't gatekeep
5. Praise good work — Not just criticism

Anti-Patterns

CODE REVIEW ANTI-PATTERNS:
✗ "LGTM" with no actual review
✗ Multi-day review latency
✗ Focusing on style over substance
✗ Demanding rewrites for preferences
✗ Only senior devs reviewing
✗ No positive feedback ever
✗ Blocking for trivial reasons

Related Solutions

• Automating code review processes
• Connecting GitHub commits to tasks automatically
• Code quality metrics and tracking