Go Code Review Skill

Rules ensuring maintainability, safety, and developer velocity. MUST rules enforced by CI/review; SHOULD rules are strong recommendations; CAN rules allowed without extra approval.

1 — Before Coding

•BP-1 (MUST): Ask clarifying questions for ambiguous requirements
•BP-2 (MUST): Draft and confirm approach before coding
•BP-3 (SHOULD): List pros/cons when multiple approaches exist
•BP-4 (SHOULD): Define testing strategy and observability signals upfront

2 — Modules & Dependencies

•MD-1 (SHOULD): Prefer stdlib; introduce dependencies with clear justification
•MD-2 (CAN): Use govulncheck for updates

3 — Code Style

•CS-1 (MUST): Enforce gofmt, go vet
•CS-2 (MUST): Avoid name stutter in packages
•CS-3 (SHOULD): Small interfaces near consumers; prefer composition
•CS-4 (SHOULD): Avoid reflection on hot paths; prefer generics
•CS-5 (MUST): Use input structs for functions with >2 arguments
•CS-6 (SHOULD): Declare input structs before consuming functions

4 — Errors

•ERR-1 (MUST): Wrap with %w and context
•ERR-2 (MUST): Use errors.Is/As for control flow
•ERR-3 (SHOULD): Define sentinel errors in package
•ERR-4 (CAN): Use context.WithCancelCause for error propagation

5 — Concurrency

•CC-1 (MUST): Sender closes channels; receivers never close
•CC-2 (MUST): Tie goroutine lifetime to context.Context
•CC-3 (MUST): Protect shared state with sync.Mutex/atomic
•CC-4 (SHOULD): Use errgroup for fan-out work
•CC-5 (CAN): Buffered channels only with rationale

6 — Contexts

•CTX-1 (MUST): ctx must be first parameter; never store in structs
•CTX-2 (MUST): Propagate non-nil ctx; honor Done/deadlines
•CTX-3 (CAN): Expose WithX helpers deriving deadlines from config

7 — Testing

•T-1 (MUST): Use GWT Given-When-Then pattern, deterministic, hermetic tests
•T-2 (MUST): Run with -race flag in CI; use t.Cleanup
•T-3 (SHOULD): Mark safe tests with t.Parallel()

8 — Logging & Observability

•OBS-1 (MUST): Structured logging with slog, levels, consistent fields
•OBS-2 (SHOULD): Correlate logs/metrics/traces via request IDs
•OBS-3 (CAN): Provide debug/pprof endpoints with auth guards

9 — Performance

•PERF-1 (MUST): Measure before optimizing using pprof, benchmarks
•PERF-2 (SHOULD): Avoid allocations on hot paths; reuse buffers
•PERF-3 (CAN): Add microbenchmarks for critical functions

10 — Configuration

•CFG-1 (MUST): Config via env/flags; validate on startup; fail fast
•CFG-2 (MUST): Treat config as immutable after initialization
•CFG-3 (SHOULD): Provide sensible defaults and clear documentation
•CFG-4 (CAN): Support hot-reload only with correctness guarantees

11 — APIs & Boundaries

•API-1 (MUST): Document exported items; minimize exported surface
•API-2 (MUST): Accept interfaces where variation needed; return concrete types
•API-3 (SHOULD): Keep functions small, orthogonal, composable
•API-5 (CAN): Use constructor options pattern for extensibility

12 — Security

•SEC-1 (MUST): Validate inputs; set explicit I/O timeouts; prefer TLS
•SEC-2 (MUST): Never log secrets; manage outside code
•SEC-3 (SHOULD): Limit filesystem/network access by default
•SEC-4 (CAN): Add fuzz tests for untrusted inputs

13 — CI/CD

•CI-1 (MUST): Lint, vet, test (-race), build on every PR
•CI-2 (MUST): Reproducible builds with -trimpath; embed version via -ldflags
•CI-3 (SHOULD): Require review sign-off for MUST rules
•CI-4 (CAN): Publish SBOM; run govulncheck/license checks

14 — Tooling Gates

•G-1 (MUST): go vet ./... passes
•G-2 (MUST): golangci-lint run passes with project config
•G-3 (MUST): go test -race ./... passes

Review Checklist

When reviewing Go code:

•Readability: Is the code clear and self-documenting?
•Complexity: Is cyclomatic complexity reasonable?
•Data structures: Are appropriate structures/algorithms used?
•Dependencies: Are hidden dependencies factored out?
•Naming: Is function/variable naming consistent?
•Error handling: Are errors wrapped with context?
•Concurrency: Are goroutines properly managed?
•Testing: Are tests table-driven and comprehensive?