# Code Review Standards & Guidelines

## Code Review Philosophy
- Why we do code reviews
- Goals: quality, knowledge sharing, consistency
- Blameless culture principles

## Review Process

### Before Submitting
- Self-review checklist
  - Code follows style guide
  - Tests are included
  - Documentation is updated
  - No debugging code left in
  - Performance implications considered

### Submitting for Review
- PR/MR template to use
- Description requirements
- How to link related work
- Who to assign as reviewers
- When to ping for urgent review

### Review Expectations
- Expected review turnaround time
- Approval requirements (number of reviewers)
- CI/CD pipeline requirements
- When you can merge (approvals needed, tests passing)

## Code Quality Standards

### Style & Formatting
- Language-specific style guide (link)
- Naming conventions
- Comment requirements
- Line length limits

### Testing Requirements
- Minimum code coverage percentage
- Unit test requirements
- Integration test requirements
- Edge case testing
- What not to test

### Documentation
- Docstring/comment requirements
- When to add/update README
- Changelog entries
- Architecture documentation updates needed

### Performance
- Performance considerations to check
- Query optimization standards
- Caching approach review points
- Load testing requirements for significant changes

### Security
- Security review checklist
- Input validation requirements
- Authentication/authorization checks
- Sensitive data handling
- When to require security team review

## Common Review Comments

### Must Fix
Items that block approval and must be addressed

**Category: Security**
- Example: "Sanitize user input before database query"
- Example: "Don't hardcode credentials"

**Category: Correctness**
- Example: "This logic doesn't handle null values"
- Example: "Off-by-one error in loop"

**Category: Performance**
- Example: "This query will cause N+1 problem"
- Example: "Consider caching this result"

### Should Fix
Items strongly recommended for improvement

- Example: "Could simplify this with list comprehension"
- Example: "This variable name is unclear"

### Nice to Have
Suggestions for improvement that can be addressed later

- Example: "Consider extracting this to a helper function"
- Example: "Could add a test for this edge case"

## Common Pitfalls to Avoid

**As a Reviewer:**
- Being nitpicky about style (use linters instead)
- Requesting changes that are stylistic preferences
- Not acknowledging good code
- Blocking on minor issues
- Not providing context for why changes are needed

**As an Author:**
- Submitting large PRs (should be < 400 lines)
- Mixing refactoring with feature work
- Not explaining the "why" in PR description
- Being defensive about feedback
- Merging without approval

## Escalation & Conflicts

### Disagreement on Approach
- When reviewer and author disagree on technical approach
- Process for resolution (discussion, escalate to tech lead, etc.)
- When to compromise vs. when to be firm

### Approval Override
- When can approvals be overridden?
- Who has authority?
- Documentation requirements

## Special Cases

### Hotfixes
- Approval requirements
- Documentation needed post-fix
- Expedited review process

### Dependency Updates
- Automatic vs. manual review
- Testing requirements
- Security patch handling

### Documentation-Only Changes
- Reduced review requirements
- Who needs to approve