Go Code Review

Go Code Review Checklist

Quick-reference checklist for reviewing Go code. Each item links to deeper guidance in the parent skill.

Naming

• MixedCaps used (no underscores)
• Initialisms are consistent case (URL, ID, HTTP)
• Variable names match scope (short for local, descriptive for global)
• Receiver names are 1-2 letters, consistent across methods
• Package names are lowercase single words, no util/common/misc
• No name repetition (package.Method, not package.PackageMethod)

Error Handling

• Errors returned, not panicked (production code)
• Error wrapping uses %w or %v appropriately
• Error context is succinct (no "failed to" chains)
• Errors handled once (not logged AND returned)
• Error strings are lowercase, no trailing punctuation
• Happy path at minimal indentation (error-first)
• Sentinel errors use Err prefix, error types use Error suffix

Concurrency

• Channel buffers are 0 or 1 (or justified)
• Goroutine lifetimes are documented
• errgroup used for error-returning goroutines
• Functions are synchronous unless concurrency is essential
• Mutexes are zero-value, unexported, not embedded in public structs
• Typed atomics used (Go 1.19+)

Testing

• Table-driven tests with named subtests
• Subtests run in parallel where safe
• go-cmp used for struct comparisons
• Failure messages include input, got, want
• t.Fatal for setup errors, t.Error for test assertions
• Interfaces defined in consumer packages

Imports

• Three groups: stdlib, external, internal
• No unnecessary renames
• No dot imports (except circular dep tests)
• Blank imports only in main/tests

Structs

• Field names used in initialization (no positional)
• Zero value fields omitted
• Types not embedded in public structs
• JSON field tags on marshaled structs

Slices and Maps

• Nil slices preferred over empty slices
• Copied at boundaries to prevent mutation
• Capacity preallocated when size is known
• Standard library slices/maps packages used

Performance

• strconv used over fmt for conversions
• No repeated string-to-byte conversions
• Map and slice capacity preallocated
• strings.Builder used for concatenation

Documentation

• Exported declarations have doc comments
• Comments are full sentences starting with declared name
• Package has package comment

Patterns

• Functional options for complex constructors
• Interface compliance verified at compile time
• defer used for resource cleanup
• Context is first parameter
• No mutable globals (dependency injection instead)
• Type assertions use two-value form
• time.Duration used instead of raw integers
• Enums start at one (zero = invalid)

Common Gotchas

• No loop variable capture bugs (Go 1.22+ or shadowed)
• Defer argument evaluation understood
• Nil interface vs nil pointer handled correctly
• Error checked before using result
• No map iteration order dependency
• Slice append backing array understood