lmiranda
4ba38eb620
Updated: - projman: 4 commands + 4 skills + integration snippet - git-flow: 4 commands + 3 skills + integration snippet - pr-review: 1 command cross-reference - cmdb-assistant: 1 command + 1 skill - data-platform: 8 commands + integration snippet - viz-platform: 11 commands + integration snippet - contract-validator: 1 command + 1 skill + 1 agent Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
150 lines
3.5 KiB
Markdown
150 lines
3.5 KiB
Markdown
---
|
|
name: review-checklist
|
|
description: Code review criteria and severity classification
|
|
---
|
|
|
|
# Review Checklist
|
|
|
|
## Purpose
|
|
|
|
Defines code review criteria, severity classification, and output format.
|
|
|
|
## When to Use
|
|
|
|
- **Code Reviewer agent**: During pre-sprint-close review
|
|
- **Commands**: `/pm-review`
|
|
|
|
---
|
|
|
|
## Severity Classification
|
|
|
|
### Critical (Must Fix Before Close)
|
|
|
|
Security issues, broken functionality, data loss risks:
|
|
|
|
- Hardcoded credentials or API keys
|
|
- SQL injection vulnerabilities
|
|
- Missing authentication/authorization checks
|
|
- Unhandled errors that could crash the application
|
|
- Data loss or corruption risks
|
|
- Broken core functionality
|
|
|
|
### Warning (Should Fix)
|
|
|
|
Technical debt that will cause problems soon:
|
|
|
|
- TODO/FIXME comments left unresolved
|
|
- Debug statements (console.log, print) in production code
|
|
- Functions over 50 lines (complexity smell)
|
|
- Deeply nested conditionals (>3 levels)
|
|
- Bare except/catch blocks
|
|
- Ignored errors
|
|
- Missing error handling
|
|
|
|
### Recommendation (Future Sprint)
|
|
|
|
Improvements that can wait:
|
|
|
|
- Missing docstrings on public functions
|
|
- Minor code duplication
|
|
- Commented-out code blocks
|
|
- Variable naming improvements
|
|
- Minor refactoring opportunities
|
|
|
|
---
|
|
|
|
## Review Patterns by Language
|
|
|
|
### Python
|
|
| Look For | Severity |
|
|
|----------|----------|
|
|
| Bare `except:` | Warning |
|
|
| `print()` statements | Warning |
|
|
| `# TODO` | Warning |
|
|
| Missing type hints on public APIs | Recommendation |
|
|
| `eval()`, `exec()` | Critical |
|
|
| SQL string formatting | Critical |
|
|
| `verify=False` in requests | Critical |
|
|
|
|
### JavaScript/TypeScript
|
|
| Look For | Severity |
|
|
|----------|----------|
|
|
| `console.log` | Warning |
|
|
| `// TODO` | Warning |
|
|
| `any` type abuse | Warning |
|
|
| Missing error boundaries | Warning |
|
|
| `eval()` | Critical |
|
|
| `innerHTML` with user input | Critical |
|
|
| Unescaped user input | Critical |
|
|
|
|
### Go
|
|
| Look For | Severity |
|
|
|----------|----------|
|
|
| `// TODO` | Warning |
|
|
| Ignored errors (`_`) | Warning |
|
|
| Missing error returns | Warning |
|
|
| SQL concatenation | Critical |
|
|
| Missing input validation | Warning |
|
|
|
|
### Rust
|
|
| Look For | Severity |
|
|
|----------|----------|
|
|
| `// TODO` | Warning |
|
|
| `unwrap()` chains | Warning |
|
|
| `unsafe` blocks without justification | Warning |
|
|
| Unchecked `unwrap()` on user input | Critical |
|
|
|
|
---
|
|
|
|
## What NOT to Review
|
|
|
|
- Style issues (assume formatters handle this)
|
|
- Architectural rewrites mid-sprint
|
|
- Issues in unchanged code (unless directly impacted)
|
|
- Bikeshedding on naming preferences
|
|
|
|
---
|
|
|
|
## Output Template
|
|
|
|
```
|
|
## Code Review Summary
|
|
|
|
**Scope**: [X files from sprint/last N commits]
|
|
**Verdict**: [READY FOR CLOSE / NEEDS ATTENTION / BLOCKED]
|
|
|
|
### Critical (Must Fix)
|
|
- `src/auth.py:45` - Hardcoded API key in source code
|
|
- `src/db.py:123` - SQL injection vulnerability
|
|
|
|
### Warnings (Should Fix)
|
|
- `src/utils.js:123` - console.log left in production code
|
|
- `src/handler.py:67` - Bare except block swallows all errors
|
|
|
|
### Recommendations (Future Sprint)
|
|
- `src/api.ts:89` - Function exceeds 50 lines, consider splitting
|
|
|
|
### Clean Files
|
|
- src/models.py
|
|
- src/tests/test_auth.py
|
|
```
|
|
|
|
---
|
|
|
|
## Verdict Criteria
|
|
|
|
| Verdict | Criteria |
|
|
|---------|----------|
|
|
| **READY FOR CLOSE** | No Critical, few/no Warnings |
|
|
| **NEEDS ATTENTION** | No Critical, has Warnings that should be addressed |
|
|
| **BLOCKED** | Has Critical issues that must be fixed |
|
|
|
|
---
|
|
|
|
## Integration with Sprint
|
|
|
|
When sprint context is available:
|
|
- Reference the sprint's issue list
|
|
- Create follow-up issues for non-critical findings
|
|
- Tag findings with appropriate labels from taxonomy
|