Code reviews are the single most effective tool for maintaining long-term software health. In the Go ecosystem, where simplicity and pragmatism are king, a bad code review process can turn a clean codebase into a tangled mess of channel deadlocks and interface pollution.
As we move through 2025, the Go standard library has evolved (hello, mature Generics and log/slog), but the fundamentals of writing robust systems remain the same. This article isn’t just about syntax—that’s what linters are for. This is a checklist for the human side of the review: logic, architecture, concurrency safety, and maintainability.
Whether you are a senior engineer mentoring a junior or a solo dev looking to discipline your commit habits, this checklist will upgrade your quality control.
Prerequisites & Environment #
Before diving into the logic, ensure your team has the automated baseline established. Human reviewers should never waste time commenting on formatting.
- Go Version: Go 1.24+ (Recommended for latest optimizations).
- Linter:
golangci-lint(The industry standard). - IDE: VS Code (with Go extension) or GoLand.
The Automated Gatekeeper #
Before you even look at the code, ensure the author has passed the automated checks. If your CI pipeline doesn’t have this, add it now.
Create a .golangci.yml in your project root:
run:
timeout: 5m
linters:
enable:
- errcheck
- gosimple
- govet
- ineffassign
- staticcheck
- unused
- bodyclose # deeply important for HTTP clients
- noctx # enforces context usageNow, let’s dive into the Manual Review Checklist.
1. Concurrency & Goroutine Leaks #
Go makes concurrency easy, which also makes shooting yourself in the foot easy. This is the highest-risk area in any Go PR.
The Lifecycle Check #
Question: Does every started goroutine have a defined way to stop?
If a goroutine starts without a context.Context or a done channel, it is a potential leak. Leaked goroutines consume memory and can hold onto file descriptors forever.
❌ Bad Practice:
func processData(data <-chan int) {
go func() {
for val := range data {
fmt.Println(val)
}
// If 'data' is never closed, this goroutine lives forever.
}()
}✅ Production Ready:
func processData(ctx context.Context, data <-chan int) {
go func() {
for {
select {
case <-ctx.Done():
// Context cancelled (timeout or shutdown), exit cleanly
return
case val, ok := <-data:
if !ok {
return // Channel closed
}
fmt.Println(val)
}
}
}()
}Race Conditions #
Question: Is shared mutable state protected?
Look for maps or slices accessed by multiple goroutines. If there isn’t a sync.Mutex or sync.RWMutex guarding writes and reads, reject the PR.
2. Error Handling and Observability #
In 2025, simply returning err isn’t enough. We need context for debugging distributed systems.
Wrapping vs. Returning #
Question: Does the error provide a stack trace of “what happened”?
Use %w to wrap errors so that errors.Is and errors.As continue to work, while adding context.
❌ Bad Practice:
if err := db.Query("SELECT..."); err != nil {
return err // Context is lost. Upper layer doesn't know WHICH query failed.
}✅ Production Ready:
if err := db.Query("SELECT..."); err != nil {
return fmt.Errorf("failed to fetch user metadata: %w", err)
}Error Handling Strategy Comparison #
Understanding when to use which error strategy is vital for API design.
| Strategy | Syntax | Use Case | Pros | Cons |
|---|---|---|---|---|
| Sentinel Errors | var ErrNotFound = ... |
Expected failures (DB, Network) | Easy to check with == or errors.Is |
Creates package coupling |
| Opaque Errors | type temporary interface { Temporary() bool } |
Decoupled libraries | Flexible, no hard dependency | Harder to implement |
| Wrapped Errors | fmt.Errorf("... %w", err) |
Application logic flow | Preserves chain, adds context | Can expose implementation details |
3. Performance & Allocations #
Go is fast, but careless coding generates garbage collection (GC) pressure.
Slice Pre-allocation #
Question: Is the slice capacity defined when the size is known?
Dynamic resizing of slices involves allocating a new array, copying data, and garbage collecting the old array.
❌ Bad Practice:
var output []User
for _, raw := range rawData {
output = append(output, parse(raw)) // Multiple re-allocations
}✅ Production Ready:
// We know the exact size needed
output := make([]User, 0, len(rawData))
for _, raw := range rawData {
output = append(output, parse(raw)) // Zero re-allocations
}Pointer vs. Value Semantics #
Question: Are pointers used only when necessary?
Passing pointers puts pressure on the Heap (Escape Analysis). Passing small structs by value is often faster because it stays on the Stack.
- Use Pointers if: The struct is large (>64 bytes typically), or you need to mutate the state.
- Use Values if: The struct is small, immutable, or a basic data container.
4. Interface Pollution #
Go interfaces should be defined by the consumer, not the producer.
Question: Is the interface strictly necessary?
Don’t create an interface UserService just because you might mock it later. Accept interfaces, return structs.
❌ Bad Practice:
package users
type UserServer interface {
Get(id string) User
Create(u User) error
Delete(id string) error
}
type Server struct {} // Implements UserServer✅ Production Ready:
Keep the Server struct definition in the package. Let the caller define the interface if they need to mock it.
// In the consumer package
type UserFetcher interface {
Get(id string) User
}
func ProcessUser(f UserFetcher, id string) {
// ...
}5. Review Workflow Visualization #
To streamline the process, follow this decision tree. This ensures you prioritize critical issues over style nits.
6. Context & Timeouts (The 2025 Standard) #
By now, almost every I/O operation should accept a context.Context.
Question: is context.Background() or context.TODO() being used inside a function?
- Rule: Contexts should be passed down the call stack, not created in the middle of it.
- Rule: Always defer the cancellation of a context if you create a derived one.
func callExternalService(ctx context.Context) error {
// Set a timeout for this specific call to prevent hanging
ctx, cancel := context.WithTimeout(ctx, 2*time.Second)
defer cancel() // CRITICAL: prevent context leak
req, _ := http.NewRequestWithContext(ctx, "GET", "http://api.internal", nil)
// ... execute request
}Conclusion #
A great code review is a balance between rigorous correctness and pragmatic delivery. By focusing on these high-impact areas—concurrency safety, error wrapping, slice allocation, and interface design—you elevate the entire team’s output.
Summary Checklist for your next PR:
- Are all goroutines guaranteed to exit?
- Is
contextpropagated through all I/O functions? - Are errors wrapped with
%w? - Are slices pre-allocated where possible?
- Are interfaces defined close to usage (consumer)?
Further Reading:
- Effective Go (The eternal classic)
- 100 Go Mistakes and How to Avoid Them by Teiva Harsanyi
- Google’s Internal Go Style Guide
Remember: Code is read 10 times more often than it is written. Make it readable.