Skip to content

code-review.md

Latest commit

 

History

History
301 lines (219 loc) · 5.94 KB

code-review.md

File metadata and controls

301 lines (219 loc) · 5.94 KB

Workflow: Code Review

Detailed process for reviewing and approving pull requests.

Overview

PR Opened → TL Spawns Reviewers → Parallel Reviews → Feedback Loop → Approval → Merge

Goal: 2 independent approvals before merge.

Review Ownership

PR Type Required Reviewers
Feature 2 reviewers + TL final
Bug fix (P2/P3) 2 reviewers + TL final
Bug fix (P0/P1) 1 reviewer OR TL only
Hotfix TL or Bogdan only
Docs only 1 reviewer

Step 1: PR Ready for Review

Coder Opens PR

gh pr create \
  --base develop \
  --title "{type}({scope}): {description}" \
  --body "Closes #{issue}
  
## Summary
{what this PR does}

## Changes
- {change 1}
- {change 2}

## Testing
{how to test}
"

gh pr edit {pr} --add-label "state:review"

TL Notified

  • TL session checks for state:review PRs
  • Or coder sends completion message to TL

Step 2: TL Spawns Reviewers

Pre-Review Check

Before spawning:

# Verify CI passes
gh pr checks {pr}

# Verify no conflicts
gh pr view {pr}

Spawn Two Reviewers

## Reviewer 1 Spawn

Label: reviewer-{project}-{pr}-1
Model: Opus

[Use prompts/code-review.md brief]
## Reviewer 2 Spawn

Label: reviewer-{project}-{pr}-2
Model: Opus

[Use prompts/code-review.md brief]

Reviewer Selection:

  1. Prefer project coders who didn't author this PR
  2. If unavailable, spawn fresh Opus reviewers
  3. Never assign author as reviewer

Step 3: Parallel Reviews

Both reviewers work independently:

┌─────────────────────┐    ┌─────────────────────┐
│  Reviewer 1         │    │  Reviewer 2         │
│  - Read CONTEXT.md  │    │  - Read CONTEXT.md  │
│  - Read PR diff     │    │  - Read PR diff     │
│  - Check checklist  │    │  - Check checklist  │
│  - Submit review    │    │  - Submit review    │
└─────────────────────┘    └─────────────────────┘
         │                          │
         └──────────┬───────────────┘
                    ↓
              Both Complete

Expected Timeline

  • Small PR (<100 lines): 15 min each
  • Medium PR (100-300 lines): 30 min each
  • Large PR (300-500 lines): 45 min each

Step 4: Handle Review Outcomes

Outcome A: Both Approve

# Add approved label
gh pr edit {pr} --add-label "state:approved" --remove-label "state:review"

Proceed to Step 5 (TL Final Review).

Outcome B: Changes Requested

Reviewer feedback
      ↓
TL notifies coder (or spawns if terminated)
      ↓
Coder addresses feedback
      ↓
Coder pushes fixes
      ↓
Coder re-requests review
      ↓
Reviewer re-reviews (same or new)

Coder Addressing Feedback:

# Coder makes fixes
git add -A
git commit -m "fix: Address review feedback

- {change 1}
- {change 2}
"
git push

# Request re-review
gh pr edit {pr} --add-assignee {reviewer}

Outcome C: Conflicting Reviews

If reviewers disagree:

  1. TL reviews the disagreement
  2. TL makes final call
  3. Document reasoning in PR comment

Outcome D: PR Too Large

If reviewer says "too large to review":

  1. TL asks coder to split PR
  2. Close current PR
  3. Open smaller, focused PRs
  4. Each smaller PR goes through full review

Step 5: TL Final Review

After 2 approvals:

TL Checks

## Final Review Checklist

- [ ] 2 approvals present
- [ ] All review comments addressed
- [ ] CI still passing
- [ ] No merge conflicts
- [ ] Conventional commit message
- [ ] PR description complete

If Issues Found

TL comments and waits for fix.

If Ready

Proceed to merge (or request Bogdan merge for sensitive changes).

Step 6: Merge

Standard Merge (TL)

# Squash merge (clean history)
gh pr merge {pr} --squash --delete-branch

Update Tracking

# Issue auto-closes via "Closes #N"

# Update CONTEXT.md Active Work (remove merged branch)
# Archive coder state file

Cleanup

# Remove worktree
git worktree remove ../{repo}-{feature}

Re-Review Protocol

When coder pushes fixes after "changes requested":

Option A: Same Reviewer

# Re-request from same reviewer
gh pr edit {pr} --add-reviewer {reviewer-username}

Option B: Fresh Reviewer

If original reviewer terminated or unavailable:

## Re-Review Spawn

Context: PR #{pr} had changes requested. Author addressed feedback.
Focus: Verify the specific issues were fixed.

[Brief includes previous review comments]

Review Metrics

Track Per Review

  • Time from PR open to first review
  • Number of review cycles
  • Time to merge after approval

Health Indicators

Metric Healthy Needs Attention
First review < 4 hours > 8 hours
Review cycles 1-2 > 3
Time to merge < 4 hours after approval > 24 hours

Common Review Issues

Issue: Nitpicking

Problem: Reviewer blocks on minor style issues
Solution: Use linter for style, review for logic

Issue: Drive-by Comments

Problem: Reviewer comments but never approves/rejects
Solution: Require explicit approve/request-changes

Issue: Rubber Stamping

Problem: "LGTM" without actual review
Solution: Require checklist completion

Issue: Review Bottleneck

Problem: PRs waiting >24h for review
Solution: Spawn more reviewers, prioritize review work

Special Cases

Self-Review (Never Allowed)

Author cannot be reviewer. System should prevent this.

TL as Author

When TL writes code:

  • 2 reviewers still required
  • Another TL or Bogdan does final review

Urgent Hotfix

  • Single reviewer (TL or Bogdan)
  • Merge immediately after approval
  • Post-merge review acceptable for P0