adam/engineering-handbook
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/engineering-handbook.git synced 2026-09-30 02:13:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions
engineering-handbook/code-review-rubric.md
Adam Moussa affa1a64f8
Some checks failed
ci / ci / ci (push) Has been cancelled
Details
docs(cd): separate terraform infra from github app deploys (#46) 
Make HCP Terraform plus GitHub Actions content CD the default for new
workloads, and keep SAM/CDK documented as the remaining path.
2026-09-15 22:05:33 +00:00

1.7 KiB
Raw Permalink Blame History

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
- ...