Go Code Review: What a Senior Will Say and Why Your Code Won't Pass

Your Go Code: From "It Works" to "Approved"

You wrote code in Go. It compiles. The tests are green. You are proud. But then a code review from a senior developer arrives, and within five minutes your pull request is covered in red comments. Sound familiar?

Go is a language where "simplicity" is deceptive. Behind the apparent ease lie dozens of pitfalls that an experienced eye sees immediately. In this article, we will break down the most frequent comments that seniors leave on Go code reviews. You will learn not only what is wrong, but also how to fix it so your code passes review on the first try.

Let's dive into typical mistakes in Golang code review.

1. Error Handling: Not Just if err != nil

Problem: Ignoring errors or a "bare" return

The most common problem for beginners is insufficiently informative error handling. Simply returning an error upward is only 50% of the work. A senior will always ask: "What went wrong? Where?"

// Bad: senior will say "add context"\nfunc ReadConfig(path string) (*Config, error) {\n    data, err := os.ReadFile(path)\n    if err != nil {\n        return nil, err // ❌ No context\n    }\n    // ...\n}\n

The correct approach is to wrap errors using fmt.Errorf and the %w placeholder (for subsequent checking via errors.Is or errors.As).

// Good: senior will nod\nfunc ReadConfig(path string) (*Config, error) {\n    data, err := os.ReadFile(path)\n    if err != nil {\n        return nil, fmt.Errorf("read config file %s: %w", path, err) // ✅ Context + chain\n    }\n    // ...\n}\n

Problem: Repetitive if err != nil

Go is known for its verbosity in error handling. But that doesn't mean you should tolerate copy-pasting. A senior will suggest extracting repetitive checks into helpers or using the "errgroup" pattern for goroutines.

\n
• Don't do this: Having if err != nil { return ... } 10 times in a row in one function is normal, but if the pattern repeats across different functions, it's time to refactor.
\n
• Senior's tip: Use defer for closing resources and checking close errors (yes, file.Close() also returns an error).
\n

2. Concurrency: Data Races and Incorrect Channels

Problem: Accessing a shared variable without synchronization

Go is a language where goroutines are cheap, but data races are expensive. A senior will immediately notice if you write to a map or slice from multiple goroutines without a mutex.

// Bad: data race\nvar counter int\nfor i := 0; i < 100; i++ {\n    go func() {\n        counter++ // ❌ Race!\n    }()\n}\n

Solution: use sync.Mutex or atomics. But a senior will say: "Wouldn't it be simpler to use channels?"

// Good: channels for synchronization\nch := make(chan int, 100)\nfor i := 0; i < 100; i++ {\n    go func() {\n        ch <- 1\n    }()\n}\nvar counter int\nfor i := 0; i < 100; i++ {\n    counter += <-ch // ✅ No races\n}\n

Problem: Goroutine leaks

Forgot to close a channel? The goroutine hangs forever. A senior on a Go code review will definitely check if you have a goroutine hanging without a select with ctx.Done().

// Bad: goroutine may hang forever\ngo func() {\n    for msg := range ch {\n        process(msg)\n    }\n}()\n// If ch is never closed — memory leak\n

The correct pattern is to always pass context.Context and listen for the cancellation signal.

// Good: leak protection\ngo func(ctx context.Context) {\n    for {\n        select {\n        case msg, ok := <-ch:\n            if !ok {\n                return // channel closed\n            }\n            process(m