/go-code-review

name: go-code-review
description: "Invoke this skill to systematically review a Go change against community style standards before merging. Walks the diff topic by topic — formatting, errors, naming, concurrency, interfaces, data structures, security, declarations, functions, style, logging, imports, generics, testing — flagging issues with line references and severity (must-fix / should-fix / nit). Apply proactively before any Go PR ships."
license: MIT
compatibility: "Designed for Claude Code or similar AI coding agents. Examples assume Go 1.21+ for log/slog references."
allowed-tools: Read Edit Write Glob Grep Bash(go:*) Bash(golangci-lint:*)

Go Code Review

A repeatable, opinionated review pass for Go code. Read the diff file by file, walk each topic in order, flag findings with file:line references, then group by severity.

Core Rules

1. **Mechanical checks first.** Never start a human review until `gofmt`, `go vet`, and `golangci-lint` are clean. They free your attention for what tools cannot catch. 2. **One file at a time, topic by topic.** Walk the diff in order and apply each topic checklist below. Switching topics mid-file loses the thread. 3. **Every finding cites a rule.** `file:line` plus the rule name (`go-naming: initialisms`) — never a bare opinion. 4. **Severity is non-negotiable.** Must-Fix (correctness/security/data-loss/broken contract) blocks merge; Should-Fix (significant design or style issue); Nit (small preference, flag once). 5. **Drop what you cannot defend.** After flagging, re-read and remove any finding you would not stand behind in a thread. 6. **Praise non-trivial improvements.** A review without acknowledgement teaches only avoidance.

Review Procedure

1. Run mechanical checks: `gofmt -d ./...`, `go vet ./...`, `golangci-lint run ./...`, `go test ./... -race -short`. 2. Read the diff one file at a time. For each file, walk the topic checklists below in order. 3. Flag every issue with `file:line` and the rule name that justifies it. 4. After all files are reviewed, re-read flagged items and drop any you cannot justify. 5. Group findings by **Must Fix / Should Fix / Nit** using the rubric below.

> Use [references/review-template.md](references/review-template.md) when writing up the review for consistent severity grouping and tone.

Automated Checks

gofmt -l ./... && go vet ./... && golangci-lint run ./... && go test ./... -race -short

Fix anything the tools find before continuing. See [go-linting](../go-linting/SKILL.md) for setup.

Formatting

• [ ] `gofmt`/`goimports` clean; long lines break by semantics, not column count

Documentation

• [ ] Exported symbols documented (starts with name, ends with `.`); package comment adjacent to `package` clause; non-trivial unexported have intent comments; named returns only when they clarify

See [go-documentation](../go-documentation/SKILL.md).

Error Handling

• [ ] No discarded errors (`_ = f()`) without a written justification
• [ ] Error strings are lowercase with no trailing punctuation
• [ ] Errors wrap with `%w` when the caller may want to inspect; `%v` only when hiding is deliberate
• [ ] No in-band magic values (`-1`, `""`, `nil`) for failure — multi-return with an `error` or `ok bool`
• [ ] Error path comes first; the success path stays unindented
• [ ] Each error is handled exactly once (log **or** return, not both)

See [go-error-handling](../go-error-handling/SKILL.md).

Naming

• [ ] `MixedCaps` / `mixedCaps` only — no underscores or `SCREAMING_SNAKE`
• [ ] Initialisms are uniformly cased: `URL`, `ID`, `HTTP`, `XMLHTTPRequest`, `serveHTTP`
• [ ] Short names for short scopes (`i`, `r`, `ctx`); longer names for wider scopes
• [ ] Receiver names are one or two letters, consistent across methods; no `this`/`self`/`me`
• [ ] Packages don't stutter (`http.Server`, not `http.HTTPServer`)
• [ ] Package names avoid `util`, `helpers`, `common`, `misc`
• [ ] No identifiers shadow builtins (`len`, `error`, `cap`, `new`, `make`, `copy`, `any`)

See [go-naming](../go-naming/SKILL.md) and [go-packages](../go-packages/SKILL.md).

Declarations

• [ ] Related `var`/`const`/`type` are in grouped blocks; unrelated kept separate
• [ ] `var` for intentional zero values; `:=` for computed locals
• [ ] Variables scoped as narrowly as is readable (if-init where it fits)
• [ ] Struct literals use field names; zero-value fields are omitted
• [ ] Enums start at `iota + 1` (or zero is explicitly meaningful)
• [ ] `any` instead of `interface{}` in new code

See [go-declarations](../go-declarations/SKILL.md).

Control Flow

• [ ] No `else` after a returning/breaking/continuing `if`; no `:=` shadowing of outer `ctx`/`err`; map iteration is order-agnostic; labeled `break`/`continue` for switch-in-loop

See [go-control-flow](../go-control-flow/SKILL.md).

Functions

• [ ] File order: type → constructor → exported → unexported → utilities; wrapped signatures one-per-line; no pointer-to-interface; bool/int params renamed via type or commented; printf-style helpers end in `f`

See [go-functions](../go-functions/SKILL.md).

Interfaces

• [ ] Defined in the consumer package; not "just for mocking" on the implementor; consistent receivers per type; compile-time `var _ I = (*T)(nil)` on exported implementations

See [go-interfaces](../go-interfaces/SKILL.md).

Concurrency

• [ ] Goroutine lifetimes clear (bounded by `ctx.Done()` or documented); APIs synchronous by default; `context.Context` first param, never struct field; lock order documented; sender closes channels

Example to flag (`pkg/worker/worker.go:42`):

go s.process(req)   // ✗ no ctx, no done signal — leak on shutdown

vs. acceptable:

s.wg.Add(1)
go func() { defer s.wg.Done(); s.process(ctx, req) }()

See [go-concurrency](../go-concurrency/SKILL.md).

Data Structures

• [ ] `var t []T` for nil slices, `[]T{}` only when empty non-nil is required (JSON output); copies of structs containing `syn