Aller au contenu

Coding standards

Ce contenu n’est pas encore disponible dans votre langue.

The conventions below exist so contributors don’t have to re-derive them each PR, and so the codebase stays readable at the 50k-line mark.

  • main is stable-only. Every commit on main is a tested, working checkpoint that can ship. Force-pushing to main is forbidden. Pushing to main directly without prior reviewed merge from dev is forbidden.
  • dev is the integration branch. Feature branches merge into dev; dev flows into main at release boundaries.
  • feat/<scope>, fix/<scope>, chore/<scope> for working branches. Delete them immediately after merge — only dev and main are long-lived.
  • Format: <area>: <imperative summary> — e.g. assets: handle 0-byte uploads, web: fix viewer crash on EXR with no exif.
  • One logical change per commit. If a refactor and a fix landed together, split them.
  • No Claude / AI co-authorship lines. The change is the contributor’s.
  • Reference the issue/PR number in the body, not the subject.
  • HEREDOC commit messages when multi-line — preserves formatting through shells.
  • Target dev, never main.
  • The PR description explains why, not what. The diff already shows what.
  • Reference any relevant ADR (See ADR 0036) and any related issue.
  • ./scripts/test.sh must be green locally before the PR opens. CI re-runs it; CI red is a blocker.
  • Squash-merge for feature branches; merge-commit for the dev → main release path.

The non-obvious rules. The obvious ones (gofmt, golint, go vet) are enforced by CI.

  • Errors return out. No silent swallows, no _ = err. If an error is genuinely safe to ignore, write a comment saying why.
  • Trust internal code, validate at boundaries. Don’t re-validate inputs your own callers already validated. Do validate at HTTP entry points (handler functions) and at external-system entry points (storage backends, connectors).
  • No defensive nil checks on guaranteed-non-nil values. If the type system or the framework guarantees a value is non-nil, don’t add a check that “can’t fire” — it makes the reader doubt the guarantee.
  • Comments explain why, not what. Code that reads as code doesn’t need narration; code that has a hidden constraint, a workaround, a non-obvious invariant, does.
  • No multi-paragraph docstrings. One short line max per exported symbol; the godoc reader is engineers who already know Go.
  • Per-domain layout. Each domain owns its internal/<domain>/ directory. Cross-domain calls go through the handler layer or through small dependency-injected interfaces, never via grabbing another domain’s repository directly.
  • DOUBLE PRECISION over NUMERIC for floats. NUMERIC maps to pgtype.Numeric in pgx, which is a pain to work with from Go.
  • COALESCE PATCH pattern for partial updates: UPDATE foo SET col = COALESCE(sqlc.narg('col'), col) and pass nil to leave the column alone.
  • Regenerate with sqlc generate after every queries.sql change. Don’t hand-edit queries.sql.go.
  • Embed-rebuild quirk — if you add a query that uses a struct embed, sometimes sqlc generate needs a clean rebuild (rm -rf app/internal/*/queries.sql.go && sqlc generate).
  • Integration tests hit a real Postgres. No DB mocks. The mock/prod divergence has bitten us before; the test script (./scripts/test.sh --go) spins up a real disposable database against which every test runs.
  • ./scripts/test.sh --go wipes admin user_roles after running. Re-INSERT the admin’s user_roles row to log in again; don’t re-run setup.
  • OpenAPI strict-server shim pattern — every domain’s test shim needs a stub for every new operation, or go vet will surface the gap.
  • No mocks for “is this even called” assertions. If you want to assert a function was called, refactor so the side effect is observable (a row in a table, a job in a queue), then test the side effect.
  • Runes in .svelte.ts files only. $state, $derived, $effect outside of a .svelte component must live in a .svelte.ts extension or they error at runtime (not compile time — surprise gotcha).
  • web/src/lib/components/ is the shared component pool. Per-route components live next to the route they belong to.
  • Type-check before push via docker compose exec -T web sh -c 'cd /app && npm run check'. Host node_modules is empty; check runs in the container.
  • Regen the API schema after openapi.yaml changes: npm run generate:api. The schema is gitignored; CI doesn’t catch staleness because CI doesn’t type-check the frontend.
  • i18n strings live in web/src/lib/i18n/<locale>.json. New surfaces add the English key; locale stubs follow.
  • No console.log in committed code. Console statements are debugger noise.

The default is no comment. Add one only when the WHY is non-obvious — a hidden constraint, a subtle invariant, a workaround for a specific bug, behaviour that would surprise a reader.

What not to write:

  • What the code does — well-named identifiers already do that.
  • References to current task / fix / callers (“used by X”, “added for the Y flow”, “handles the case from issue #123”) — belongs in the PR description and rots in the codebase.
  • Multi-paragraph commentary — if a thing needs three paragraphs to explain, it probably needs an ADR.

Run ./scripts/test.sh and ensure green before any push. The script:

  • Brings up the full Docker Compose stack (Postgres, app, web).
  • Runs the Go integration tests against the real DB.
  • Runs the frontend type-check via the web container.
  • Cleans up.

A pre-merge build failure on CI almost always reflects a step skipped locally; the local script is faster than waiting for CI and shows the same failures.

Migrations live in app/internal/db/migrations/ as goose-managed NNNNN_*.sql files. The schema in app/schema.sql mirrors the migration end-state; sqlc validates queries against the mirror at code-generation time.

  • Pre-MVP: baseline squashes are allowed under ADR 0046. The squash branch is chore/migration-baseline-vN, ships an audit report (docs/cleanup-audit-<date>.md), and replaces the prior sequence with a single baseline migration. No upgrade path from the old sequence is supported — developers docker compose down -v after pulling.
  • Post-v1.0: forward-only forever. No more squashes. Real operators with real data depend on goose up working from any prior shipped version.
  • CHECK constraints mirror typed Go constants per ADR 0042. When the migration enforces a closed set at the DB layer, the same set lives as typed constants in the owning package; both halves land in the same PR.
  • Update app/schema.sql in the same commit that adds a migration. The two never drift; reviewers compare them on the spot.

Source code (Go, Svelte, SQL comments, OpenAPI) should not lean on direct named comparisons to competitor or inspiration products. Reasons: reduces legal exposure if a vendor takes issue, ages badly when the named product pivots, and confuses future readers who don’t know what “AFFiNE-shaped” means.

What this looks like in practice:

  • Don’t write “Mastodon-shaped activity dispatch” → do write “ActivityPub-shaped activity dispatch”.
  • Don’t write “the prior generation’s resource_node table” → do state the design on its own terms.
  • Don’t write “AFFiNE’s element-typed model” → do write “an element-typed model”.
  • Don’t write “Frame.io-style timestamped comments” → do write “asset-anchored timestamped comments”.
  • Don’t write “PeerTube has to move every byte” → do drop the comparison; just describe what we do.
  • Don’t keep rs_user_id / rs_resource_id column names — rename to neutral external_user_id or drop if the column is now unused.

Exemptions. Three places retain named upstream / competitor references intentionally:

  • ADRs and docs/research/ — decision documents and historical record.
  • License attribution surfaces — third-party dependency notices required by their licenses (acknowledgment is not comparison). The former ResourceSpace fork-base attribution was removed once the RS-derived code was deleted and the project relicensed to AGPL-3.0 (ADR 0001 → superseded by ADR 0040).
  • Diff / comparison tooling (scripts/rs-diff.sh, scripts/gen-rs-baseline.py) — these scripts exist to compare against the upstream fork base; naming the target in the script’s own header comment is operational, not editorial.

Everything else in source stays neutral.

  • Backwards-compatibility shims for code that no real caller depends on. If you rename a function and nothing real consumed the old name, delete the old name. Don’t keep “for compat” zombies.
  • Legacy fallback paths in wired handlers. Per ADR 0044, every handler that emits federation activities uses WithEmission / WithEmissionFn as its single code path. The “if the activities writer isn’t wired, fall back to the domain write” pattern is forbidden: in production it never fires; in tests it hides the federation-delivery correctness invariant. Tests wire the activities writer in setup; there is no second path.
  • Test stubs that proliferate per-package. OpenAPI strict-server tests use the shared internal/openapi/strictservershim.PanicShim. Embed it; override only the methods the test actually exercises. Do not create a new per-package shim file with N panic methods every time an OpenAPI operation lands.
  • Speculative abstractions. Three similar lines beats a premature abstraction for two. The third caller motivates the helper; the second doesn’t.
  • --no-verify on commits. Pre-commit hooks exist for a reason. If a hook fails, fix the underlying problem.
  • --force-recreate workarounds for stale containers in production paths. It’s fine for local dev when Go binary changes don’t pick up; production has different lifecycle rules.

Read recently-merged PRs to see how the current contributor base writes code. Reference the conventions ADR (0035) for documentation patterns. If the answer’s still unclear, ask in the PR or in an issue — the project lead would rather answer once than untangle ten divergent patterns later.