majiayu000 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): Table-driven, 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?