mirror of
https://github.com/Sea-Haven-Industries/engineering-handbook.git
synced 2026-09-30 08:03:16 +00:00
37 lines
976 B
Markdown
37 lines
976 B
Markdown
|
# Code Review Rubric
|
||
|
|
|
||
|
|
## Finding Categories
|
||
|
|
|
||
|
|
| Category | Meaning | Action Required |
|
||
|
|
|---|---|---|
|
||
|
|
| BLOCK | Must fix before merge | Yes -- PR cannot merge |
|
||
|
|
| FIX | Should fix, strong recommendation | Yes -- unless explicitly deferred with issue |
|
||
|
|
| NIT | Optional improvement, style preference | No -- author's discretion |
|
||
|
|
| QUESTION | Needs clarification before reviewer can assess | Yes -- answer required |
|
||
|
|
|
||
|
|
## Review Checklist
|
||
|
|
|
||
|
|
- Correctness: does the code do what the PR says?
|
||
|
|
- Security: OWASP top 10, secrets handling, input validation at boundaries
|
||
|
|
- Sea Haven conventions: naming, secrets placement, Lambda defaults, IaC patterns
|
||
|
|
- Operational readiness: logging, error handling, monitoring, CI/CD
|
||
|
|
- Tests: appropriate coverage for the change
|
||
|
|
|
||
|
|
## Output Format
|
||
|
|
|
||
|
|
```
|
||
|
|
Verdict: APPROVE | REQUEST CHANGES | NEEDS DISCUSSION
|
||
|
|
|
||
|
|
### BLOCK
|
||
|
|
- **file.py:42** -- [problem]. Why it matters: [impact]. Fix: [suggestion].
|
||
|
|
|
||
|
|
### FIX
|
||
|
|
- ...
|
||
|
|
|
||
|
|
### NIT
|
||
|
|
- ...
|
||
|
|
|
||
|
|
### QUESTION
|
||
|
|
- ...
|
||
|
|
```
|