Skip to content

Contributing

Jump to bottom
“Jesse edited this page Apr 7, 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
typos Spell check staged files (uses .typos.toml config)
gofmt Ensures all Go files are formatted (excludes _templ.go)
templ freshness Verifies generated _templ.go files match their .templ sources
OpenAPI reminder Warns if API handlers changed without spec update
go build Compiles all packages to catch build errors
golangci-lint Runs the full linter suite (see below)
govulncheck Scans dependencies for known vulnerabilities
hadolint Dockerfile best-practice linting

These hooks are installed via make hooks from .githooks/pre-commit. An alternative .pre-commit-config.yaml exists for users of the pre-commit framework.

Go file guard: gofmt, templ, go build, golangci-lint, and govulncheck are skipped automatically when no .go, go.mod, .go.sum, or .templ files are staged. Docs-only and config-only commits complete instantly.

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

Linting

The project uses golangci-lint v2 with 14 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)
nilerr Catches returning nil when err is non-nil
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.

Repository mocks: The artist.Service supports dependency injection via NewServiceWithRepos(...). For unit tests that do not need a real database, implement the repository interfaces (Repository, ProviderIDRepository, ImageRepository, MemberRepository, AliasRepository, PlatformIDRepository) as test doubles and inject them. For integration tests that exercise SQL queries, use setupTestDB with real SQLite instead.

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: scripts/smoke.sh is the primary integration smoke test. It runs against a live instance, exercises all major route groups (50+ checks), and exits non-zero if any check fails. Run it before opening a PR whenever you change API handlers, auth logic, or platform integration code:

# Default (admin/admin against http://localhost:1973):
bash scripts/smoke.sh

# Custom credentials/instance:
SW_USER=admin SW_PASS=yourpassword SW_BASE=http://localhost:1973 bash scripts/smoke.sh

# Include destructive Tier 4 checks (image fetch, backup create):
SW_USER=admin SW_PASS=yourpassword bash scripts/smoke.sh --full

The script authenticates once (session cookie), mints a short-lived sw_ API token, uses it for all subsequent requests (bypassing CSRF), and revokes the token on exit. IDs are discovered dynamically from the live DB -- no hardcoded values to maintain.

Bruno collections in api/bruno/ exist for manual, exploratory API work. The smoke script is the authoritative automated test; Bruno collections may be out of date.

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. Run the local review toolkit before pushing:

    /pr-review-toolkit:review-pr

    Copilot reviews only the diff on each push, so every unfixed finding produces another round of review comments. Running the toolkit locally first collapses this into a single pass. Fix all critical and important findings, then commit, before opening the PR.

  4. 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)

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

  6. 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

Stillwater uses a single migration file. To change the schema:

  1. Edit internal/database/migrations/001_initial_schema.sql directly
  2. Add or modify tables/columns in the appropriate -- +goose Up section
  3. Update the corresponding -- +goose Down section
  4. Test locally: go test ./internal/database/...
  5. Migrations run automatically on application startup via goose

Important: Do not create new migration files. All schema changes go into 001_initial_schema.sql.

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