Code Review in Go: What to Check and How to Give Feedback in Practice

Introduction: Why Code Review in Go Is Not Just a Checkbox

Code review is not a bureaucratic procedure, but a key element of development culture. In Go, with its concise syntax and strict rules, review often becomes the first filter that catches problems early on. Many teams transitioning to Golang find that old habits (e.g., from Python or PHP) don't work: Go forgives fewer type-related errors but requires attention to detail.

In practice, code review in Go solves three tasks: team training (newcomers adapt to the code style faster), quality improvement (finding bugs and anti-patterns), and code consistency (so the project doesn't become a "patchwork quilt"). Let's break down what exactly to look for and how to formulate comments.

1. What to Check First: From Syntax to Errors

Any review starts with the basics. In Go, there are several "hot spots" worth checking automatically or manually.

1.1. Error Handling

Go does not use exceptions — errors are returned as values. A common problem: a developer ignores the returned error using _. This is only acceptable in rare cases (e.g., fmt.Println).

// Bad: error is ignored\nresp, _ := http.Get("https://api.example.com/data")\n\n// Good: error is handled\nresp, err := http.Get("https://api.example.com/data")\nif err != nil {\n    log.Printf("failed to get data: %v", err)\n    return err\n}\ndefer resp.Body.Close()

What to check:

\n
• Are all errors handled (not just _)?
\n
• Is defer used to close resources (files, connections)?
\n
• Are errors in goroutines being "swallowed"?
\n

1.2. Naming and Style (Go Idioms)

Go requires brevity: camelCase for private, PascalCase for exported. Avoid "Hungarian notation" and long names.

// Bad\nfunc GetUserDataFromDatabase(userID int) (*UserData, error)\n\n// Good\nfunc GetUser(id int) (*User, error)

Red flags:

\n
• Variable names like temp, data, val (too generic).
\n
• Functions longer than 30-40 lines (need to be broken up).
\n
• Missing comments for exported functions (this is a golint requirement).
\n

1.3. Goroutines and Synchronization

This is the most complex part. Errors in concurrent code are a frequent source of bugs in production.

// Bad: data race\nvar counter int\nfor i := 0; i < 100; i++ {\n    go func() {\n        counter++ // unsafe\n    }()\n}\n\n// Good: use a mutex\nvar mu sync.Mutex\nvar counter int\nfor i := 0; i < 100; i++ {\n    go func() {\n        mu.Lock()\n        counter++\n        mu.Unlock()\n    }()\n}

What to check:

\n
• Is there a sync.Mutex or atomic for shared data?
\n
• Was the channel closed (close(ch))?
\n
• Are goroutines leaking (no select with a timeout)?
\n

2. Architectural Checks: From Interfaces to Packages

After basic syntax, look at the code structure. Go encourages composition, not inheritance.

2.1. Interfaces: Small and on the Consumer Side

In Go, interfaces are implemented implicitly. A good interface contains 1-3 methods. Check if the author is trying to create a "comprehensive" interface.

// Bad: huge interface\ntype Storage interface {\n    Save(data []byte) error\n    Load(id string) ([]byte, error)\n    Delete(id string) error\n    List() ([]string, error)\n    Connect() error\