4c0898cd49
Builds out the user-facing docs tree alongside the slice-1 build target. Kept the implementer's planned Task 7 'Slice 1 dev loop' README section untouched — these docs are the canonical destination for that pointer. - docs/QUICKSTART.md: 5-min path to 'hear the echo' (libopus install, cargo run, browser steps, troubleshooting, what's happening under the hood). - docs/DEVELOPMENT.md: dev loop — workspace layout, per-crate iteration, running tests, the 20 ms loop / 'drop + observe' rule, slice-1 boundaries (what NOT to add yet). - CONTRIBUTING.md (at repo root, conventional): trunk-based dev, CI gates, commit message style, atomic commits, code style + learner-facing documentation policy, terminology policy, PR workflow + review checklist, GPL-3.0-or-later license. - README.md: add a Quickstart pointer at the top, a Documentation table linking to every doc, and the slice-1 build-target status block.
216 lines
7.7 KiB
Markdown
216 lines
7.7 KiB
Markdown
# Contributing
|
|
|
|
Thanks for considering a contribution to Rutster. This file is the short
|
|
form — read [`AGENTS.md`](AGENTS.md) for the full orientation any agent
|
|
(human, AI, hybrid) working in the repo should have.
|
|
|
|
---
|
|
|
|
## Trunk-based development
|
|
|
|
1. Branch from `main` for any change.
|
|
2. Open a PR targeting `main`.
|
|
3. CI gates (below) must pass before merge.
|
|
4. Squash-merge to keep `main` linear.
|
|
|
|
**Never push directly to `main`.** Branch protection is planned to
|
|
enforce this; until then, self-discipline.
|
|
|
|
### Branch naming
|
|
|
|
Single-purpose branches named after the change:
|
|
|
|
- `slice-1-webrtc-loopback` — long-running build-target branch
|
|
- `docs/quickstart` — documentation additions
|
|
- `media/opus-roundtrip-fix` — bug fix with crate scope
|
|
- `ci/deny-toml` — CI config
|
|
|
|
No strict scheme — short, descriptive, hyphen-separated.
|
|
|
|
---
|
|
|
|
## CI gates
|
|
|
|
All four must pass before merge to `main`:
|
|
|
|
```bash
|
|
cargo fmt --check # formatting
|
|
cargo clippy -- -D warnings # lints (warnings = failures)
|
|
cargo test --all # all unit + integration tests
|
|
cargo deny check # licenses, advisories, bans, sources
|
|
```
|
|
|
|
CI runs on push + PR to `main`, matrix: latest stable + the MSRV pinned
|
|
in `rust-toolchain.toml`. The `clippy` + `test` jobs install `libopus-dev`
|
|
(the one system dependency, per PORT_PLAN §7).
|
|
|
|
If your change adds a new dependency, run `cargo deny check` locally
|
|
before pushing — a license conflict or duplicate-version ban will fail
|
|
CI, and it's faster to catch locally.
|
|
|
|
---
|
|
|
|
## Commit messages
|
|
|
|
- **Imperative mood**: "Add X" / "Fix Y", not "Added X" / "Fixes Y".
|
|
- **Subject ≤ 72 chars**.
|
|
- **Body wraps at 72**, blank line between subject and body.
|
|
- **Reference ADRs / specs** by number when relevant: `ADR-0002`,
|
|
`slice-1 spec §3.4`.
|
|
- Match the style of recent commits:
|
|
```bash
|
|
git log --oneline -10
|
|
```
|
|
|
|
Example shape (from the repo history):
|
|
|
|
```
|
|
media: PcmFrame + AudioSource/Sink + Opus codec pair
|
|
|
|
PcmFrame is the canonical tap format (16-bit mono @ 24 kHz, 480 samples
|
|
per 20 ms frame — ARCHITECTURE.md). AudioSource/AudioSink are the seam
|
|
step 2 splices the tap client into (spec §3.3); EchoAudioPipe is the
|
|
slice-1 wiring of that seam. OpusDecoder/OpusEncoder wrap the opus
|
|
crate's libopus FFI with hot-path match-and-continue.
|
|
```
|
|
|
|
### Atomic commits
|
|
|
|
One logical change per commit. Doc ratifications, code, and tests each
|
|
land as separate commits when practical. Don't bundle unrelated work.
|
|
|
|
A new feature + its tests can land in one commit if the test is part of
|
|
the feature's correctness story (TDD). A refactor + a new feature should
|
|
be two commits.
|
|
|
|
### Never commit secrets
|
|
|
|
`.gitignore` covers `.env*`, `*.pem`, `*.key`. If a new secret pattern
|
|
appears in your work, extend `.gitignore` in the same commit.
|
|
|
|
---
|
|
|
|
## Code style
|
|
|
|
See [`AGENTS.md`](AGENTS.md) "Code style" for the full guide. Highlights:
|
|
|
|
- `cargo fmt` is the source of truth for formatting. Don't hand-format.
|
|
- `clippy -D warnings` is the lint bar. Fix the code, don't suppress
|
|
with `#[allow]` unless the rationale is documented inline.
|
|
- `snake_case` (functions, variables, modules, crates), `PascalCase`
|
|
(types), `UPPER_SNAKE_CASE` (constants).
|
|
- Newtype wrappers over primitives for type-safety — e.g. `ChannelId(Uuid)`,
|
|
not bare `Uuid`.
|
|
- Hot path (the 20 ms media loop): **never** `?`-propagate.
|
|
Match-and-continue. Dropped packet must not terminate the peer.
|
|
- Cold path: `thiserror`-derived error enums + `?`, converted to HTTP
|
|
status codes at the axum boundary.
|
|
- Never `unwrap()` / `expect()` outside tests or const-init contexts.
|
|
|
|
### Documentation comments (learner-facing — important)
|
|
|
|
**This project overrides the default "no comments" convention.** The
|
|
user is learning Rust from this codebase. Code in slice 1 (and the
|
|
spearhead steps) carries thorough educational comments:
|
|
|
|
- `//!` module docs on every `lib.rs` / `main.rs` / sub-module
|
|
- `///` item docs on every public struct / enum / fn / trait
|
|
- `//` inline comments on the *mechanism*, not the what — why this
|
|
ownership pattern, why `Arc<Mutex<>>` vs `Arc<RwLock<>>`, why an
|
|
`enum` over a struct with a `kind` field, etc. Aim: a Rust learner
|
|
reads the comment and learns a specific Rust concept they wouldn't
|
|
have inferred from the code alone.
|
|
|
|
`LEARNING.md` at the repo root indexes "to learn concept X, read file Y"
|
|
pointers. Add to it when you introduce a new pattern worth surfacing.
|
|
|
|
This verbosity is a deliberate trade-off: more tokens now, compound
|
|
educational value later. Once a pattern is established, later slices can
|
|
be sparser on the well-trodden patterns.
|
|
|
|
---
|
|
|
|
## Slice-1 boundaries — what NOT to add (yet)
|
|
|
|
If an agent (you, an AI pair, a contributor) proposes adding any of these
|
|
in slice 1, the right answer is "no, see slice-1 spec §1.2":
|
|
|
|
- Dedicated timing thread for the media loop (step 4)
|
|
- TLS on the HTTP signaling surface (step 5)
|
|
- Authn / authz / multi-tenancy (step 6)
|
|
- Trickle ICE (when NATs demand it)
|
|
- The tap itself (step 2 — slice 1 only *pre-paves* the seam)
|
|
- The brain / STT / LLM / TTS (step 3)
|
|
- Barge-in / VAD-driven playout kill (step 4)
|
|
- PSTN trunk / SIP client (step 5)
|
|
- Spend cap / abuse gate (step 6)
|
|
- Browser-based automated e2e tests / Selenium / Playwright (post-slice-1)
|
|
- Docker / compose (later rung)
|
|
- Event bus / Valkey / CDR emission (step 5)
|
|
- Transfer / park / pickup / barge features (escalation rung 2)
|
|
|
|
The spearhead depends on this sequencing. Adding things early breaks
|
|
the ordering each step is its own proof.
|
|
|
|
---
|
|
|
|
## Terminology policy
|
|
|
|
**Avoid authoritarian / exclusionary terms** in code, prose, identifiers,
|
|
and endpoint names. Use equally-descriptive alternatives:
|
|
|
|
| Avoid | Use instead |
|
|
|---|---|
|
|
| police / policing (the verb) | enforce / gate / guard |
|
|
| master / slave | primary / replica, leader / follower, controller / worker |
|
|
| blacklist / whitelist | denylist / allowlist, blocklist / safelist |
|
|
| officer | operator / handler / controller |
|
|
| censor | suppress / filter |
|
|
|
|
**Exception: protocol-convention names are kept verbatim** when they come
|
|
from upstream specs or libraries — replacing them would hurt the
|
|
educational mapping to upstream docs. **ICE** (Interactive Connectivity
|
|
Establishment, RFC 8445) stays: it's the protocol name in `str0m::ice`,
|
|
`RTCIceCandidate`, and the cargo ecosystem.
|
|
|
|
See [`AGENTS.md`](AGENTS.md) "Terminology policy" for the full table.
|
|
|
|
---
|
|
|
|
## Pull request workflow
|
|
|
|
1. Branch from `main`.
|
|
2. Make atomic commits per the guidance above.
|
|
3. Push the branch + open a PR targeting `main`.
|
|
4. CI runs: fmt, clippy, test, deny. All four must be green.
|
|
5. Reviewer checks the diff against the spec / ADRs cited in the commit
|
|
message. New dependencies require scrutiny — `cargo deny check`
|
|
enforces license + source bans, but reviewers should also sanity-check
|
|
the dependency choice against the architecture.
|
|
6. Squash-merge once approved + green.
|
|
7. Delete the branch post-merge (keeps the branch list tidy).
|
|
|
|
### Reviewing
|
|
|
|
When reviewing a PR:
|
|
|
|
- Does it cite the relevant ADR / spec section? (For substantial changes.)
|
|
- Does it add anything from the slice-1 "what NOT to add" list? (Reject
|
|
if so — refer to slice-1 spec §1.2.)
|
|
- Are educational comments present where a new pattern is introduced?
|
|
- Any `unwrap()` / `expect()` outside tests? (Reject unless justified.)
|
|
- Does the hot-path code use `?`? (Reject — it must match-and-continue.)
|
|
- Does the diff bundle unrelated work? (Ask for split commits.)
|
|
|
|
---
|
|
|
|
## License
|
|
|
|
By contributing, you agree your contributions are licensed under
|
|
**GPL-3.0-or-later** (ADR-0004). Strong copyleft in the Asterisk lineage.
|
|
|
|
If you contribute code with a different license header / SPDX expression
|
|
in a Cargo manifest, CI will reject it (`cargo deny check licenses`).
|
|
Don't introduce dependencies whose licenses conflict — check `deny.toml`
|
|
for the allow-list.
|