Skip to content

Contributing

Jump to bottom
Jesse Slaton edited this page Feb 28, 2026 · 7 revisions

Contributing

Guidelines for contributing to Stillwater. This page covers code style, commit conventions, the PR process, and how to extend the codebase.

Code Style

Formatting:

  • All Go code must pass gofmt (enforced by pre-commit hook)
  • Templ files must pass templ fmt (run via make fmt)
  • No trailing whitespace, files end with a newline

Conventions:

  • No emoji in code, commits, comments, or documentation
  • No em-dashes in any output
  • Structured logging via log/slog -- never fmt.Println or log.Printf
  • Wrap errors with context: fmt.Errorf("loading artist %d: %w", id, err)
  • Use context.Context as the first parameter for any function that does I/O
  • Prefer returning errors over panicking
  • Keep functions short and focused; extract helpers when a function exceeds ~50 lines

Naming:

  • Follow standard Go naming conventions (MixedCaps, not snake_case)
  • Package names are lowercase, single-word where possible
  • Avoid stuttering: artist.Artist is fine, artist.ArtistService is acceptable, but artist.ArtistArtist is not
  • HTTP handlers are methods on a handler struct, named after the action: ListArtists, GetArtist, UpdateArtist

Commit Conventions

All commits must follow Conventional Commits. This is enforced by a pre-commit hook.

Format:

<type>: <description>

[optional body]

[optional footer]

Allowed types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert

Examples:

feat: add Deezer provider for artist metadata
fix: prevent duplicate NFO writes on concurrent requests
docs: update wiki Architecture page with event bus diagram
refactor: extract rate limiter map into shared utility
test: add table-driven tests for image resolution checks

GPG signing: All commits must be GPG-signed. The pre-commit hook does not enforce this, but unsigned commits will be rejected by branch protection rules.

Pre-commit Hooks

Pre-commit hooks run automatically before each commit. Install them with:

make hooks

What the hooks check:

Hook What it does
trailing-whitespace Removes trailing whitespace
end-of-file-fixer Ensures files end with a newline
check-yaml Validates YAML syntax
check-added-large-files Rejects files over 500KB
check-merge-conflict Detects unresolved merge conflict markers
gitleaks Scans for accidentally committed secrets
golangci-lint Runs the full linter suite (see below)
gofmt Ensures all Go files are formatted
conventional-pre-commit Validates commit message format (commit-msg stage)

If a hook fails, fix the issue and re-stage your changes before committing again.

Linting

The project uses golangci-lint v2 with 13 linters enabled:

Linter What it catches
errcheck Unchecked error returns
govet Suspicious constructs (printf format strings, struct tags, etc.)
staticcheck Advanced static analysis (deprecated APIs, unreachable code, etc.)
unused Unused variables, functions, types
bodyclose Unclosed HTTP response bodies
gosec Security issues (SQL injection, weak crypto, etc.)
noctx HTTP requests without context
sqlclosecheck Unclosed SQL rows and statements
unconvert Unnecessary type conversions
unparam Unused function parameters
wastedassign Assignments that are never read
misspell Common spelling mistakes (US English)
revive Style and correctness rules

Excluded from test files: gosec, errcheck (test code is allowed to be less strict about error checking and security patterns).

Run locally:

make lint         # or: golangci-lint run ./...

Configuration is in .golangci.yml at the repository root.

Testing

Running tests:

make test              # all tests with race detector
go test ./...          # basic run (no race detector)
go test -v ./internal/rule/...   # single package, verbose

Test database: Tests use in-memory SQLite databases. The setupTestDB helper (found in various _test.go files) creates a fresh database, runs migrations, and returns a ready-to-use *sql.DB. Each test gets its own database instance -- no shared state between tests.

Context helpers: Use WithTestUserID(ctx, userID) to set a test user ID in the context for handlers that require authentication.

Table-driven tests: The preferred pattern for testing multiple cases:

tests := []struct {
    name    string
    input   SomeInput
    want    SomeOutput
    wantErr bool
}{
    {name: "valid input", input: ..., want: ...},
    {name: "missing field", input: ..., wantErr: true},
}
for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        got, err := Function(tt.input)
        if (err != nil) != tt.wantErr {
            t.Fatalf("unexpected error: %v", err)
        }
        if got != tt.want {
            t.Errorf("got %v, want %v", got, tt.want)
        }
    })
}

Testdata fixtures: Some packages include a testdata/ directory with sample NFO files, images, or other fixtures used by tests. These are committed to the repository and referenced via relative paths in test code.

API testing: Bruno collections in api/bruno/ provide manual API test suites. Install Bruno, open the collection, and run requests against a running instance.

PR Process

  1. Branch from main: Use descriptive branch names like feat/deezer-provider or fix/nfo-duplicate-write.

  2. Keep PRs focused: One logical change per PR. If a feature requires multiple steps, consider stacking PRs or breaking the work into sequential PRs.

  3. CI checks: Every PR runs these checks automatically:

    • golangci-lint (same config as local)
    • go test -race ./...
    • go build (ensures the binary compiles)
    • Docker image build (ensures the container builds)

  4. Review: All PRs require at least one review before merging. Address feedback by pushing new commits (do not force-push or amend during review).

  5. Merge: Squash-merge is the default. The PR title becomes the commit message, so write it as a conventional commit (e.g., "feat: add Deezer provider").

Database Migrations

To add a new migration:

  1. Create a SQL file in internal/database/migrations/:

    YYYYMMDDHHMMSS_description.sql

    Example: 20260227120000_add_webhook_retries.sql

  2. Add both up and down sections:

    -- +goose Up
ALTER TABLE webhooks ADD COLUMN max_retries INTEGER NOT NULL DEFAULT 3;

-- +goose Down
ALTER TABLE webhooks DROP COLUMN max_retries;

  3. Test locally:

    go test ./internal/database/...

  4. Migrations run automatically on application startup. There is no separate migration command.

Naming convention: Use snake_case descriptions that describe the change: add_webhook_retries, create_api_tokens_table, drop_legacy_settings.

Adding a Provider

To add a new metadata provider:

  1. Create the provider file in internal/provider/:

    internal/provider/<name>.go

  2. Implement the Provider interface:

    type Provider interface {
    Name() ProviderName
    RequiresAuth() bool
    SearchArtist(ctx context.Context, name string) ([]ArtistSearchResult, error)
    GetArtist(ctx context.Context, id string) (*ArtistMetadata, error)
    GetImages(ctx context.Context, id string) ([]ImageResult, error)
}

  3. Add the provider name to the ProviderName constants and AllProviderNames slice in internal/provider/provider.go.

  4. Register a rate limiter in the RateLimiterMap (see internal/provider/ratelimit.go). Set the rate according to the provider's API policy.

  5. Add capability metadata describing the provider's access tier, help URL for API key registration, and rate limit info.

  6. Wire it up in main.go: Instantiate the provider with its dependencies (HTTP client, rate limiter, API key from encryption service) and register it with the Registry.

  7. Add tests in internal/provider/<name>_test.go. Use a mock HTTP client to avoid hitting real APIs in tests.

  8. Seed default provider settings in a new database migration so existing installations get sensible defaults for the new provider.

Clone this wiki locally