Design Review

Design Review

Perform a comprehensive design review of the Go codebase. Evaluate code quality, naming conventions, consistency, and architectural patterns. Produce a scored report with actionable improvements.

When to use

Use this skill when:

• Auditing overall design quality before a major refactor or release
• Onboarding to an unfamiliar Go codebase and need a structured assessment
• Comparing codebase against Go idioms after a period of rapid development

Do NOT use for:

• Reviewing a single PR or uncommitted diff (use code-review instead)
• Writing new code or fixing bugs
• Running linters/tests for CI gating (use code-review instead)

Relationship to code-review

Dimensions

Score each dimension 1-10 and provide specific file:line references for issues found.

1. Naming Conventions

• Exported types/functions follow Go conventions (MixedCaps, no underscores)
• Package names are short, lowercase, singular (not utils, helpers, common)
• Interface names use -er suffix where appropriate (Reader, Writer)
• Receiver names are short (1-2 chars), consistent within type
• Variable names: short in small scopes, descriptive in large scopes
• Acronyms are all-caps (ID, URL, HTTP, not Id, Url, Http)
• Test function names follow TestFuncName_Scenario pattern
• Constants use MixedCaps, not SCREAMING_SNAKE

2. Package Design

• Packages have clear, singular responsibility
• No circular dependencies
• Internal packages used appropriately for implementation details
• Package-level doc comments present
• No God packages (too many responsibilities)
• Reasonable file sizes (flag files > 500 lines)

3. Interface Design

• Interfaces are small (1-3 methods preferred)
• Interfaces defined where consumed, not where implemented
• No unnecessary interface pollution (concrete types are fine)
• Accept interfaces, return structs
• io.Reader, io.Writer, fmt.Stringer used where applicable

4. Error Handling

• Errors wrapped with context using fmt.Errorf("...: %w", err)
• Custom error types where callers need to inspect errors
• No swallowed errors (unchecked err returns)
• Sentinel errors are var ErrFoo = errors.New(...) not string comparison
• Error messages are lowercase, no punctuation, no "failed to" prefix

5. Concurrency Patterns

• Goroutines have clear ownership and lifecycle
• Channels used for communication, mutexes for state
• context.Context propagated correctly
• No goroutine leaks (all goroutines have exit paths)
• sync.WaitGroup or errgroup used for fan-out

6. API Consistency

• Similar operations use similar signatures across packages
• Config structs vs option functions used consistently
• Constructor functions follow NewFoo pattern
• Consistent use of pointer vs value receivers within a type
• Method ordering: constructor, public methods, private methods

7. Code Organization

• One primary type per file (type + methods)
• Test files mirror source files (foo.go / foo_test.go)
• Constants and vars at top of file
• init() functions avoided (or justified)
• Build tags and platform files follow conventions

8. Documentation

• Exported functions have doc comments starting with function name
• Package doc comments present
• Complex algorithms have inline comments explaining why
• No stale/misleading comments
• Examples in tests for complex APIs

Procedure

1. Set scope first:

   • Determine target scope from command args (/design-review, package path, or --focus)
   • Apply default include/exclude rules from the Scope section
   • Note any skipped areas explicitly in the report

2. Scan structure: Use Glob (**/*.go) to map the package layout. Use Shell with go list ./... to enumerate packages and wc -l for line counts.

3. Automated checks: Run in parallel:

   • go vet ./... — catch common mistakes
   • golangci-lint run ./... (if available) — extended lint checks
   • Check for //nolint directives and their justifications
   • If a tool is unavailable, continue review and mark that check as "skipped"

4. Naming audit: Use Grep to sample each package:

   • Search for naming violations: pattern Id[^s] instead of ID, Url[^s] instead of URL
   • Use Read to check receiver name consistency per type
   • Verify exported function doc comments
   • Check package names against conventions

5. Interface audit: Use Grep for type \w+ interface to find all interface definitions:

   • Count methods per interface (flag > 5 methods)
   • Check if interfaces are defined at consumer or producer
   • Look for interface embedding depth

6. Error handling audit: Sample error paths:

   • Grep for unchecked errors: _, _ = or bare function calls
   • Check error wrapping: %w vs %v vs %s
   • Look for string-based error checking vs sentinel/type errors

7. Consistency audit: Compare patterns across packages:

   • Constructor patterns: Grep for func New across all packages and compare signatures
   • Config patterns (struct vs options)
   • Logging patterns (structured vs printf)
   • Test patterns (table-driven vs individual)

8. Cross-reference: Use tooling for efficiency:

   • Dead code: Grep for unexported function definitions, then Grep for call sites within the package
   • Duplicates: compare function signatures across packages
   • Inconsistent patterns doing the same thing differently

9. Validate report before output: Self-check the report against these gates:

   • Every scored dimension has at least one finding with a file:line reference, or an explicit "no issues found" note
   • Every Top 5 item has a before/after code snippet or an exact command to run
   • Key Strengths section has at least 2 entries with file references
   • Overall score matches the weighted average (recalculate to confirm)
   • No dimension is scored without evidence — if you can't find evidence, mark N/A If any gate fails, fix the report before presenting it.