mirror of
https://github.com/Sea-Haven-Industries/engineering-handbook.git
synced 2026-09-30 03:23:14 +00:00
affa1a64f8
Some checks failed
ci / ci / ci (push) Has been cancelled
Make HCP Terraform plus GitHub Actions content CD the default for new workloads, and keep SAM/CDK documented as the remaining path.
48 lines
1.7 KiB
Markdown
48 lines
1.7 KiB
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
|
|
- HCP app CD, when the PR touches `terraform/` or `deploy-*.yaml`: stub plus `ignore_changes` present, GitHub Actions does not create HCP runs, plan role has no `Get*` wildcards, prod Environment allows `main` and `v*`
|
|
- Tests: appropriate coverage for the change
|
|
|
|
### HCP app CD BLOCK
|
|
|
|
Treat as BLOCK when the PR touches `terraform/` or `deploy-*.yaml` and any of these are true:
|
|
|
|
- Terraform would revert app content (missing bootstrap stub or missing `ignore_changes` on the attributes GitHub Actions writes)
|
|
- A GitHub Actions workflow creates or applies an HCP run
|
|
- The plan role uses `Get*` wildcards
|
|
- The prod Environment omits `v*` or `main` from its deployment branch policy
|
|
- `hcp_iam.tf` or an in-repo `hcptf-*` role is added (those belong in org-baseline)
|
|
- A workflow cuts the GitHub Release with `GITHUB_TOKEN`
|
|
|
|
## Output Format
|
|
|
|
```
|
|
Verdict: APPROVE | REQUEST CHANGES | NEEDS DISCUSSION
|
|
|
|
### BLOCK
|
|
- **file.py:42** -- [problem]. Why it matters: [impact]. Fix: [suggestion].
|
|
|
|
### FIX
|
|
- ...
|
|
|
|
### NIT
|
|
- ...
|
|
|
|
### QUESTION
|
|
- ...
|
|
```
|