Go Code Review

Review Workflow

Follow this sequence to avoid false positives and catch version-specific issues:

1. Check

   go.mod

   — Note the Go version. This determines which patterns apply (loop variable capture is only an issue pre-1.22,

   slog

   is available from 1.21,

   errors.Join

   from 1.20). Skip version-gated checks that don't apply.
2. Scan changed files — Read full functions, not just diffs. Many Go bugs hide in what surrounds the change.
3. Check each category — Work through the checklist below, loading references as needed.
4. Verify before reporting — Load beagle-go:review-verification-protocol before submitting findings.

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

Review Checklist

Error Handling

• All errors checked (no

  _ = err

  without justifying comment)
• Errors wrapped with context (

  fmt.Errorf("...: %w", err)

  )

• errors.Is

  /

  errors.As

  used instead of string matching

• errors.Join

  used for aggregating multiple errors (Go 1.20+)
• Zero values returned alongside errors

Concurrency

• No goroutine leaks (context cancellation or shutdown signal exists)
• Channels closed by sender only, exactly once
• Shared state protected by mutex or sync types
• WaitGroups used to wait for goroutine completion
• Context propagated through call chain
• Loop variable capture handled (pre-Go 1.22 codebases only)

Interfaces and Types

• Interfaces defined by consumers, not producers
• Interface names follow

  -er

  convention
• Interfaces minimal (1-3 methods)
• Concrete types returned from constructors

• any

  preferred over

  interface{}

  (Go 1.18+)
• Generics used where appropriate instead of

  any

  or code generation

Resources and Lifecycle

• Resources closed with

  defer

  immediately after creation
• HTTP response bodies always closed
• No

  defer

  in loops without closure wrapping

• init()

  functions avoided in favor of explicit initialization

Naming and Style

• Exported names have doc comments
• No stuttering names (

  user.UserService

  →

  user.Service

  )
• No naked returns in functions > 5 lines
• Context passed as first parameter

• slog

  used over

  log

  for structured logging (Go 1.21+)

Severity Calibration

Critical (Block Merge)

• Unchecked errors on I/O, network, or database operations
• Goroutine leaks (no shutdown path)
• Race conditions on shared state (concurrent map access without sync)
• Unbounded resource accumulation (defer in loop, unclosed connections)

Major (Should Fix)

• Errors returned without context (bare

  return err

  )
• Missing WaitGroup for spawned goroutines

• panic

  for recoverable errors
• Context not propagated to downstream calls

Minor (Consider Fixing)

• interface{}

  instead of

  any

  in Go 1.18+ codebases
• Missing doc comments on exports
• Stuttering names
• Slice not preallocated when size is known

Informational (Note Only)

• Suggestions to add generics where code generation exists
• Refactoring ideas for interface design
• Performance optimizations without measured impact

When to Load References

• Reviewing error return patterns → error-handling.md
• Reviewing goroutines, channels, or sync types → concurrency.md
• Reviewing type definitions, interfaces, or generics → interfaces.md
• General review (resources, naming, init, performance) → common-mistakes.md

Valid Patterns (Do NOT Flag)

These are acceptable Go patterns — reporting them wastes developer time:

• _ = err

  with reason comment — Intentionally ignored errors with explanation
• Empty interface /

  any

  — For truly generic code or interop with untyped APIs
• Naked returns in short functions — Acceptable in functions < 5 lines with named returns
• Channel without close — When consumer stops via context cancellation, not channel close
• Mutex protecting struct fields — Even if accessed only via methods, this is correct encapsulation

• //nolint

  directives with reason — Acceptable when accompanied by explanation
• Defer in loop — When function scope cleanup is intentional (e.g., processing files in batches)
• Functional options pattern —

  type Option func(*T)

  with

  With*

  constructors is idiomatic

• sync.Pool

  for hot paths — Acceptable for reducing allocation pressure in performance-critical code

• context.Background()

  in main/tests — Valid root context for top-level calls

• select

  with

  default

  — Non-blocking channel operation, intentional pattern
• Short variable names in small scope —

  i

  ,

  err

  ,

  ctx

  ,

  ok

  are idiomatic Go

Context-Sensitive Rules

Only flag these issues when the specific conditions apply:

go.mod

go.mod

log

Before Submitting Findings

Load and follow review-verification-protocol before reporting any issue.