itsnex1s code-review
Guidelines for requesting and performing code reviews effectively.
Core Principle
Review early, review often - catch issues before they escalate.
When to Request Review
Mandatory Reviews
- After completing individual tasks in workflows
- After finishing major features
- Before merging to main branch
- After significant refactoring
Recommended Reviews
- Every 3 tasks in a longer workflow
- When unsure about implementation approach
- After adding new dependencies
- When touching security-sensitive code
How to Request Review
Get Commit Context
# Recent commits
git log --oneline -10
# Diff from main
git diff main..HEAD
# Changed files
git diff --name-only main..HEAD
# Specific commit
git show <commit-hash>
Provide Context
When requesting review, include:
- What changed and why
- Areas of concern or uncertainty
- Testing done
- Related issues/PRs
Review Checklist
Functionality
- Code does what it's supposed to do
- Edge cases are handled
- Error handling is appropriate
- No obvious bugs or logic errors
Code Quality
- Code is readable and self-documenting
- Functions/methods are focused (single responsibility)
- No unnecessary complexity
- DRY - no significant duplication
- Naming is clear and consistent
Security
- No hardcoded secrets
- Input validation present
- SQL injection prevention
- XSS prevention
- Auth/permissions checked
Performance
- No N+1 queries
- No unnecessary re-renders (React)
- Expensive operations are optimized
- Caching used where appropriate
Testing
- Tests cover new functionality
- Tests cover edge cases
- Existing tests still pass
- Test names are descriptive
Style & Conventions
- Follows project coding standards
- Consistent with surrounding code
- Linting passes
- TypeScript types are appropriate
Severity Levels
🔴 Critical (Must Fix Now)
- Security vulnerabilities
- Data loss potential
- Crashes or major bugs
- Breaking changes without migration
Action: Stop and fix immediately
🟡 Important (Fix Before Merge)
- Logic errors
- Missing error handling
- Performance issues
- Missing tests for critical paths
Action: Resolve before proceeding
🟢 Minor (Can Fix Later)
- Style inconsistencies
- Non-critical improvements
- Documentation gaps
- Refactoring suggestions
Action: Create follow-up task or address if quick
Responding to Feedback
Prioritization
- Fix all critical issues immediately
- Address important issues before advancing
- Note minor issues for follow-up
Challenging Feedback
You may push back on feedback if:
- You have technical justification
- The suggestion contradicts project patterns
- The change would introduce more complexity
Explain your reasoning clearly.
Common Review Comments
Code Smells
- Function too long (>50 lines) - consider splitting
- Too many parameters (>4) - consider options object
- Deep nesting (>3 levels) - consider early returns
- Magic numbers - use named constants
- Commented-out code - remove or explain
React Specific
- Missing dependency in useEffect
- Inline function in render causing re-renders
- Missing key prop in lists
- State that should be derived
- Prop drilling - consider context
TypeScript Specific
- Using 'any' type - be more specific
- Missing return type annotation
- Non-null assertion (!) without justification
- Type assertion without validation
Review Commands
GitHub CLI
# Request review
gh pr edit --add-reviewer username
# View PR diff
gh pr diff <number>
# Check PR status
gh pr checks <number>
# Approve PR
gh pr review --approve
# Request changes
gh pr review --request-changes --body "Please address..."
# Comment
gh pr review --comment --body "Looks good overall..."
Git Commands for Review
# Checkout PR locally
gh pr checkout <number>
# View changes
git diff main...HEAD
# View specific file changes
git diff main...HEAD -- path/to/file.ts
# View commit messages
git log main..HEAD --oneline
Anti-Patterns to Avoid
As Reviewer
- ❌ Nitpicking style when linter should catch it
- ❌ Rewriting code to personal preference
- ❌ Blocking on minor issues
- ❌ Vague feedback ("this looks wrong")
As Author
- ❌ Skipping review because "it's simple"
- ❌ Ignoring critical feedback
- ❌ Proceeding with unresolved important issues
- ❌ Dismissing valid technical feedback
- ❌ Large PRs that are hard to review
Best Practices
For Authors
- Keep PRs small and focused (<400 lines)
- Self-review before requesting
- Provide context in PR description
- Respond to all comments
- Be open to feedback
For Reviewers
- Review promptly (within 24h)
- Be constructive, not critical
- Explain the "why" not just "what"
- Praise good code too
- Use suggestions feature for fixes
Integration with Development
Batching Reviews
- Review every 3 tasks in longer workflows
- Balance thoroughness with velocity
- Don't let reviews pile up
Automated Checks
- Run linting before review
- Run tests before review
- Use automated code analysis tools
Attribution
Inspired by obra/superpowers