dtourolleandClaude Opus 5
95847e3a31
NFR-COMPAT-2 asks for the v1 channels to be stated, and says why in its own second sentence: the channel decision and the storage design are coupled. There was nowhere that statement lived. packaging/ held a PKGBUILD and a desktop entry, which is a recipe rather than a decision, and the coupling the requirement points at was therefore invisible. docs/distribution.md states five channels and, more usefully, which two of them exist only as promises. It also records what every channel has to get right independently of format — the one identifier that appears in four places, the metainfo, Vulkan being a requirement rather than a preference while NFR-R8 is open, a secrets daemon being optional rather than required, and the LFS pointer check that stops a package shipping 130 bytes where an 11 MB model should be. §4 is the part worth reading. Preparing a Flatpak is what surfaced that FR-PLAT-LIN-3 is not satisfied and cannot be satisfied by packaging alone: a folder library is chosen by typing an absolute path into an EndpointOnly field that checks it with std::fs, and nothing in the tree calls the FileChooser portal. Inside a sandbox that path does not exist, so the launch screen refuses it. Import fails one step earlier, because a sandboxed process reads its own mount namespace and a card mounted on the host is not in it. That is written down rather than fixed with --filesystem=host, and the argument for not fixing it that way is §2: the Arch package and an AppImage both hand the application the same unrestricted process the developer runs it in, so Flatpak is the only Linux channel that tests whether a design assumed unrestricted access. Granting the permission removes the only reason to ship it. The reverse coupling on Android is recorded too. NFR-COMPAT-2 says Play distribution is what makes ARCH §6.9 binding; §6.9 is verified rather than assumed, so SAF is already unconditional and a sideloaded build would gain nothing by asking for more. Play is deferred over the GPLv3 question, which is a licence-reading exercise and blocks nothing in the storage design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
165 lines
6.7 KiB
Markdown
165 lines
6.7 KiB
Markdown
# Contributing to DarkRoom
|
|
|
|
There is a lot of documentation here — 14 documents and 177 numbered
|
|
requirements — and almost all of it is written for someone who has already
|
|
decided to work on this. This file is the other thing: how to get a first
|
|
change landed without reading any of it.
|
|
|
|
## The shortest useful contribution
|
|
|
|
**A develop operation is one file.** Not one file plus a registration, plus a
|
|
shader edit, plus a control in the UI — one file:
|
|
|
|
```
|
|
core/dr-pipeline/ops/split_toning.yaml
|
|
```
|
|
|
|
`build.rs` finds it with `read_dir`, compiles it into Rust implementing
|
|
`Operation`, and from there it is indistinguishable from a hand-written node.
|
|
It arrives with controls built from its declared parameter kinds, a place in
|
|
the chain from `order:`, a place in the panel from `attributes:`, sidecar
|
|
persistence, and its own tests — which are declared in the same file and run
|
|
under `cargo test`.
|
|
|
|
Read [`core/dr-pipeline/ops/README.md`](core/dr-pipeline/ops/README.md) and
|
|
copy [`exposure.yaml`](core/dr-pipeline/ops/exposure.yaml). Split toning,
|
|
colour zones, selective colour and channel-mixer variants are all pure point
|
|
operations, which means all of them are declarations rather than code.
|
|
|
|
If you want to understand one thing about the architecture before starting,
|
|
make it this: **the core describes its capabilities and the interface composes
|
|
them.** No code in `ui/` names an operation, and a test enforces that
|
|
(`ui/dr-ui/tests/ui_names_no_operation.rs`). It is why your node needs no UI
|
|
change.
|
|
|
|
## Getting it to build
|
|
|
|
**Git LFS is required.** Model weights are stored in LFS, and a clone made
|
|
without it leaves a ~130-byte text pointer where an 11 MB model should be:
|
|
|
|
```bash
|
|
git lfs install && git lfs pull
|
|
```
|
|
|
|
Forget this and `dr-segment`'s build script stops with an instruction rather
|
|
than embedding the pointer and failing at inference time — but it is easier to
|
|
run the two commands now.
|
|
|
|
**The toolchain pins itself.** `rust-toolchain.toml` selects 1.92.0 and rustup
|
|
fetches it on first use. Do not override it; `cargo fmt` and `clippy` are both
|
|
version-sensitive and CI runs exactly this version.
|
|
|
|
**System packages.** Slint and winit need these at build time. On Debian or
|
|
Ubuntu:
|
|
|
|
```bash
|
|
sudo apt-get install pkg-config libfontconfig1-dev libxkbcommon-dev
|
|
```
|
|
|
|
**Then:**
|
|
|
|
```bash
|
|
cargo run -p darkroom-desktop
|
|
```
|
|
|
|
The first build resolves 826 crates and takes a while — on a laptop, long
|
|
enough to look like a hang. It is not one.
|
|
|
|
Android is a containerised toolchain and is not needed for most work; see
|
|
[`docker/android/README.md`](docker/android/README.md) if you get there.
|
|
|
|
## What CI will check
|
|
|
|
All four of these run on every push, so run them before you send anything:
|
|
|
|
```bash
|
|
cargo fmt --all -- --check
|
|
cargo clippy --workspace --all-targets -- -D warnings
|
|
cargo test --workspace
|
|
cargo build --workspace --release
|
|
```
|
|
|
|
GPU tests skip themselves where there is no adapter rather than failing — a
|
|
test that cannot run is not evidence either way — so a green run on a machine
|
|
without a GPU is expected, and does not mean the GPU paths were exercised.
|
|
|
|
## Requirements and traceability
|
|
|
|
[`requirements.md`](docs/requirements.md) is the register of record.
|
|
[`traceability.md`](docs/traceability.md) is generated from `TRACES:` tags in
|
|
the source and must never be hand-edited:
|
|
|
|
```rust
|
|
// TRACES: FR-DEV-3a | FR-DEV-3c
|
|
```
|
|
|
|
Tags are read from `.rs`, `.slint`, `.wgsl` and `.yaml` — the last so a
|
|
declared operation can record the requirement it satisfies, since the Rust it
|
|
generates lands in `OUT_DIR` and is not scanned.
|
|
|
|
A pre-commit hook regenerates the matrix and stages it whenever you touch
|
|
something that can carry a tag, so you should not have to think about it. If
|
|
you do need to run it by hand:
|
|
|
|
```bash
|
|
cargo run -p traceability -- report
|
|
```
|
|
|
|
Note that it tracks line numbers, so a change that only moves code still moves
|
|
the matrix. Never regenerate it with a stale prebuilt binary.
|
|
|
|
**One convention that the tooling cannot enforce.** A tag proves that a tag
|
|
exists, not that the code under it does the thing — `docs/code-health.md`
|
|
CH-4 has the details, and two requirements currently read as covered on the
|
|
strength of plumbing a future feature would use. So: **close a requirement
|
|
with a test that would fail if the behaviour were removed.** Coverage that
|
|
moves slowly and means something beats coverage that moves quickly.
|
|
|
|
## Two invariants the build defends
|
|
|
|
Worth knowing before you trip one, because both failures name a requirement
|
|
rather than a line:
|
|
|
|
- **No operation may be named in `ui/`** (FR-DEV-3a). Special-casing one
|
|
operation in the panel to fix a layout problem is how a generated interface
|
|
stops being generated. If a node needs presentation the panel cannot give it,
|
|
the answer is a `presentation:` hint in the declaration and a `WidgetKind`,
|
|
not a branch in `develop.rs`.
|
|
- **The operation schema rejects ambiguity at build time**: a duplicate
|
|
`order:`, a filename disagreeing with its `id:`, a default outside its own
|
|
range, an expression naming something that is not a parameter. Each error
|
|
names the key you got wrong and exits rather than panicking.
|
|
|
|
## Commit messages
|
|
|
|
Imperative subject describing the change from the reader's side — "Offer the
|
|
merge when two people turn out to share a name", not "fix: merge dialog". No
|
|
conventional-commits prefixes.
|
|
|
|
The body is where the reasoning goes, and it is expected to be substantial when
|
|
the change is. This codebase records *why* far more than most, in commits and
|
|
in comments alike, and that is the single habit most worth adopting: the
|
|
constraint you worked around is invisible to whoever reads the diff next.
|
|
|
|
One commit per change. If you fixed two things, that is two commits.
|
|
|
|
## Where to read next, in order
|
|
|
|
| Document | Read it when |
|
|
|---|---|
|
|
| [`core/dr-pipeline/ops/README.md`](core/dr-pipeline/ops/README.md) | Adding or changing a develop operation — start here regardless |
|
|
| [`docs/architecture.md`](docs/architecture.md) | Anything touching the render path, catalog or sync |
|
|
| [`docs/code-health.md`](docs/code-health.md) | Deciding what to work on; grades each seam by what it costs |
|
|
| [`docs/technical-debt.md`](docs/technical-debt.md) | Something looks wrong — check it was not chosen |
|
|
| [`docs/distribution.md`](docs/distribution.md) | Packaging a build, or adding a permission to one |
|
|
| [`docs/requirements.md`](docs/requirements.md) | Reference, not reading |
|
|
|
|
`technical-debt.md` is the one to check before "fixing" anything surprising.
|
|
It records compromises that were deliberate, each with the reasoning and a
|
|
falsifiable condition for when it stops being one — the point being that you
|
|
can tell a constraint from an accident without asking.
|
|
|
|
## Licence
|
|
|
|
GPL-3.0-or-later. By contributing you agree your work is licensed the same way.
|