Files
everything-claude-code/rules/common/code-review.md
xingzihai da74f85c10 fix: address review feedback from PR #929
- Add missing code-review.md and development-workflow.md to zh/README.md directory listing
- Add mkdir -p command before copy in manual install instructions
- Fix TypeScript test command path in SKILL-DEVELOPMENT-GUIDE.md
- Add Anti-Patterns section to SKILL.md template
- Add Template category to Skill Categories table in CONTRIBUTING.md
- Add Pre-Review Requirements section to code-review.md (both en and zh)
- Add Pre-Review Checks step to development-workflow.md (both en and zh)
- Add trailing newlines to all files that were missing them
2026-03-26 04:37:08 +00:00

125 lines
3.4 KiB
Markdown

# Code Review Standards
## Purpose
Code review ensures quality, security, and maintainability before code is merged. This rule defines when and how to conduct code reviews.
## When to Review
**MANDATORY review triggers:**
- After writing or modifying code
- Before any commit to shared branches
- When security-sensitive code is changed (auth, payments, user data)
- When architectural changes are made
- Before merging pull requests
**Pre-Review Requirements:**
Before requesting review, ensure:
- All automated checks (CI/CD) are passing
- Merge conflicts are resolved
- Branch is up to date with target branch
## Review Checklist
Before marking code complete:
- [ ] Code is readable and well-named
- [ ] Functions are focused (<50 lines)
- [ ] Files are cohesive (<800 lines)
- [ ] No deep nesting (>4 levels)
- [ ] Errors are handled explicitly
- [ ] No hardcoded secrets or credentials
- [ ] No console.log or debug statements
- [ ] Tests exist for new functionality
- [ ] Test coverage meets 80% minimum
## Security Review Triggers
**STOP and use security-reviewer agent when:**
- Authentication or authorization code
- User input handling
- Database queries
- File system operations
- External API calls
- Cryptographic operations
- Payment or financial code
## Review Severity Levels
| Level | Meaning | Action |
|-------|---------|--------|
| CRITICAL | Security vulnerability or data loss risk | **BLOCK** - Must fix before merge |
| HIGH | Bug or significant quality issue | **WARN** - Should fix before merge |
| MEDIUM | Maintainability concern | **INFO** - Consider fixing |
| LOW | Style or minor suggestion | **NOTE** - Optional |
## Agent Usage
Use these agents for code review:
| Agent | Purpose |
|-------|---------|
| **code-reviewer** | General code quality, patterns, best practices |
| **security-reviewer** | Security vulnerabilities, OWASP Top 10 |
| **typescript-reviewer** | TypeScript/JavaScript specific issues |
| **python-reviewer** | Python specific issues |
| **go-reviewer** | Go specific issues |
| **rust-reviewer** | Rust specific issues |
## Review Workflow
```
1. Run git diff to understand changes
2. Check security checklist first
3. Review code quality checklist
4. Run relevant tests
5. Verify coverage >= 80%
6. Use appropriate agent for detailed review
```
## Common Issues to Catch
### Security
- Hardcoded credentials (API keys, passwords, tokens)
- SQL injection (string concatenation in queries)
- XSS vulnerabilities (unescaped user input)
- Path traversal (unsanitized file paths)
- CSRF protection missing
- Authentication bypasses
### Code Quality
- Large functions (>50 lines) - split into smaller
- Large files (>800 lines) - extract modules
- Deep nesting (>4 levels) - use early returns
- Missing error handling - handle explicitly
- Mutation patterns - prefer immutable operations
- Missing tests - add test coverage
### Performance
- N+1 queries - use JOINs or batching
- Missing pagination - add LIMIT to queries
- Unbounded queries - add constraints
- Missing caching - cache expensive operations
## Approval Criteria
- **Approve**: No CRITICAL or HIGH issues
- **Warning**: Only HIGH issues (merge with caution)
- **Block**: CRITICAL issues found
## Integration with Other Rules
This rule works with:
- [testing.md](testing.md) - Test coverage requirements
- [security.md](security.md) - Security checklist
- [git-workflow.md](git-workflow.md) - Commit standards
- [agents.md](agents.md) - Agent delegation