Claude Opus 5andbdeshi
9d817dcadf
Naming is free before a module is published, a URL is shared, or a binary is deployed; every day it waits costs more. Swept every form: module path, binary, cmd/ directory, KHOSRA_SITE, the feature-loop skill directory, and the prose in earlier ADRs — which describe this project under its old name, not a different project. Recorded as ADR-0030. go mod init lands here rather than with the first feature because the module path is what the rename is about. x/text and yaml.v3 are required but not yet imported, so both are indirect and no direct dependency is claimed yet.
105 lines
6.8 KiB
Markdown
105 lines
6.8 KiB
Markdown
# Conventions
|
||
|
||
The style floor. Do not ask about anything here — read it and comply. Disagreement is legitimate but
|
||
goes through an ADR, not a diff.
|
||
|
||
## Language and dependencies
|
||
- Go, current stable release. Stdlib first, every time.
|
||
- `net/http`, `html/template`, `log/slog`, `os`, `io/fs`, `embed`. Routing is `http.ServeMux` with Go
|
||
1.22+ patterns — `GET /{section}/{slug}/` and `{path...}` for multi-segment slugs — so no router
|
||
library is needed. No web framework, ORM, or config library — flags plus environment variables, parsed in one place. The site root
|
||
(`-site`, `KHOSRA_SITE`) is the only required setting; default templates are `embed`ded so a bare
|
||
site root renders (ADR-0011).
|
||
- New dependency = ADR + human approval + `scripts/allowed-deps.txt`. `verify.sh` enforces it.
|
||
- Prefer 40 lines of obvious code over a dependency doing it in one call — unless the 40 lines would be
|
||
subtly wrong in cases the author cannot predict, which is why YAML is a dependency (ADR-0020).
|
||
|
||
## Package layout
|
||
```
|
||
cmd/khosra/ main, flag parsing, explicit wiring — the only place things are assembled
|
||
internal/content/ bundles, frontmatter, slugs, queries — knows the disk, not HTTP
|
||
internal/render/ markdown, transforms, templates — knows content, not HTTP
|
||
internal/web/ handlers, routing, headers, caching — knows both, exposes neither
|
||
internal/ext/ extensions, one package each — earned at its counter (see extensions.md)
|
||
```
|
||
Dependencies point inward. `internal/content` imports nothing from the others; `cmd` imports everything
|
||
and is imported by nothing. A feature under `internal/ext/` may import `internal/content` and
|
||
`internal/render`, and must not import `internal/web`, `cmd/`, or **another feature** — sibling imports
|
||
are what make an agent's read set compound (ADR-0027). Every `internal/ext/*` package carries a `doc.go`;
|
||
`verify.sh` fails without one. Routes reach `web` by assembly in `cmd/khosra/wire.go`, so `web` never
|
||
learns features exist. No `utils`, `helpers`, `common`, `shared`, `manager`, `base`,
|
||
`impl`, `core` — a package name not describing a domain is a smell. Flat until a package exceeds
|
||
`FILE_LOC_WARN`; do not pre-partition, and a single-file package therefore warns at the same point it
|
||
wants splitting.
|
||
|
||
## Naming and shape
|
||
- Functions under `FUNC_LOC_WARN`, ideally under 20. Nesting depth under 4. File length is not a
|
||
target: one coherent file beats two split to satisfy a counter. Values: `scripts/budgets.env`.
|
||
- No `init()`. No package-level mutable state. No singletons. Wire explicitly in `cmd`.
|
||
- Accept interfaces only where a second implementation exists; return concrete types.
|
||
- `ctx context.Context` first when a call can block or be cancelled — not decoratively.
|
||
- Never call `time.Now()` outside a `clock.go`. A render that reads the clock is only true for a while,
|
||
and an injected accessor is what makes that expressible later; `verify.sh` fails on any other caller,
|
||
because a forgotten expiry serves staleness silently.
|
||
- Comments explain *why*, never *what*. Delete a comment narrating the next line. One stating a
|
||
non-obvious invariant is worth ten describing control flow.
|
||
|
||
## Documentation
|
||
|
||
Written for a maintainer working alone, years from now, with no agent to explain anything. `verify.sh`
|
||
fails on a missing package comment and on a citation naming an ADR that does not exist; a missing doc
|
||
comment on an exported identifier is a warning, because no gate can tell `// Load loads.` from a useful
|
||
sentence, and a gate whose cheapest satisfaction is noise buys noise.
|
||
|
||
- **Every package has a package comment.** What it owns, what it does not, and which packages may import
|
||
it. For `internal/ext/*` that is the four-line `doc.go` shape in `extensions.md`.
|
||
- **Every exported identifier has a doc comment** (warned, not gated). The exported surface of a
|
||
2000-line core is small, and
|
||
`go doc ./...` is the only navigation tool that still works when nothing else does. A comment that
|
||
restates the name (`// Load loads.`) is worse than none: say what it returns on absence, what it costs,
|
||
what it assumes.
|
||
- **Cite the ADR where the decision lives in the code.** `// Path shape: ADR-0008.` at the point that
|
||
builds a URL, `// Visibility inherits: ADR-0024.` at the guard. Twenty-seven decisions are unreachable
|
||
from code otherwise, and the next maintainer changes something whose reasoning they never saw.
|
||
`verify.sh` fails on a citation naming an ADR that does not exist.
|
||
- **Comment the trap, not the mechanism.** NFC normalisation, the settle window, parse order for template
|
||
overrides, the `old ∪ new` membership rule — each is a line of code that looks arbitrary and is not.
|
||
Those are the comments worth writing.
|
||
|
||
## Errors
|
||
- Wrap with `%w` at package boundaries, with operation and path: `parse %s: %w`.
|
||
- Never log and return the same error. Handle it, or return it.
|
||
- Request-time render failure degrades: log, serve what exists, never 500 on a missing field.
|
||
Startup failure is fatal and loud. Content authoring errors name the file and line.
|
||
|
||
## Tests
|
||
- **Behaviour ships with a test.** A change to `cmd/` or `internal/` carries a `_test.go` change in
|
||
the same commit; `verify.sh` fails otherwise. Test the observable contract, not coverage for its own
|
||
sake — one test proving the new behaviour is enough, and a refactor with no behaviour change needs
|
||
only the existing tests to still pass (touch them or say why not).
|
||
- Table-driven. Golden files in `testdata/`, regenerated behind a `-update` flag.
|
||
- Test the observable contract: URL in → bytes out; file on disk → page struct. Do not test private
|
||
helpers, and do not add a seam solely to make something testable.
|
||
- Needing a mock means the design is probably wrong — use a `testdata` directory with
|
||
`os.DirFS`/`fstest.MapFS`.
|
||
- One end-to-end test per route beats ten unit tests of render internals.
|
||
|
||
## Performance
|
||
Correct and small first; fast where measured. The render path is the only hot path.
|
||
- Write to `io.Writer`, never build pages by string concatenation.
|
||
- Parse templates once at startup; never per request.
|
||
- No `sync.Pool`, caching layer, or goroutines in the render path until a benchmark justifies it and
|
||
the number goes in the commit message.
|
||
- No reflection in the hot path. `Extra` lookups are map reads, not reflection.
|
||
- Benchmarks live next to what they measure, added only when a decision depends on them.
|
||
|
||
## Frontend output
|
||
Semantic HTML working with zero JavaScript. Progressive enhancement only. No build step for CSS.
|
||
Page-specific styles and scripts come from the bundle (`styles`/`scripts` frontmatter). Respect
|
||
`prefers-reduced-motion`. Every image gets width, height, and alt. Payload discipline is a feature of
|
||
this project, not an optimisation.
|
||
|
||
## Git
|
||
One feature per commit. Imperative subject under 72 characters; body says *why*. Doc updates ride in
|
||
the same commit as the code that made them true.
|