From 91d11514b492ca660a333a7ee76c6199d0d76ab3 Mon Sep 17 00:00:00 2001 From: Anthony Minessale II Date: Thu, 13 Aug 2026 12:38:40 -0500 Subject: [PATCH 1/2] docs: add CONTRIBUTING.md and a PR template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same handful of mistakes keeps arriving from different people: pushing without running the gates, tests that assert nothing, new test functions without annotations, `__all__` appended instead of sorted, and work duplicated because nobody checked the open PRs. Every one of those is caught by `scripts/run-ci.sh`, which nothing told anyone to run. Neither repo had a CONTRIBUTING.md or a PR template. Two things worth calling out, because a contributor cannot discover either: - Installing `requirements-dev.txt` is load-bearing beyond running the tests. mypy's answer depends on which packages are importable — a `# type: ignore` that is required with sentence-transformers installed is an unused-ignore error without it — so skipping the install grades different code than CI. - Pull requests from forks cannot run CI at all. They receive no repository secrets, so the setup step that needs one fails in seconds with "Input required and not supplied: token". That is not something the contributor can fix, and it looks exactly like a broken patch. The doc says to run the gates locally and note it; a maintainer re-runs from a branch. Deliberately does not restate the engineering rules — those are enforced by the gates, and the fuller ruleset lives in infrastructure an outside contributor cannot see. Anything requiring that repo (the surface oracle) is written as "flag it, a maintainer lands it" rather than as a task. Master copy: the other nine ports carry a language-adapted version. --- .github/pull_request_template.md | 38 ++++++++++ CONTRIBUTING.md | 116 +++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 .github/pull_request_template.md create mode 100644 CONTRIBUTING.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..1ccca72d --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,38 @@ + + +## What this changes + + + +## Checklist + +- [ ] `bash scripts/run-ci.sh` passes locally +- [ ] New tests assert on content (not just "does not raise" / "is not None") +- [ ] New test functions are type-annotated (`mypy` covers `tests/`) + +## Does this change public API? + +- [ ] No +- [ ] Yes — naming it here so a maintainer can land the matching + infrastructure change: + + + +## Changing something that goes on the wire? + +If this alters an emitted shape, an enum value, or a parameter name, say where +you confirmed it — the generated types under `signalwire/**/*_generated.py` are +derived from the engine's schemas and are the authority here. + + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..003ba5eb --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,116 @@ +# Contributing + +> This is the master copy. The other SignalWire SDK ports carry a +> language-adapted version of the same document. + +## The one thing + +```bash +bash scripts/run-ci.sh +``` + +Run it before you push. It runs the same gates CI does, in the same order, and +it is the difference between a review about your change and a review about +formatting. + +Most of what follows is just explaining what that command already checks. + +## Setup + +```bash +pip install -e . +pip install -r requirements-dev.txt +``` + +`requirements-dev.txt` is not optional, and not only for running the tests. +The type checker's answer depends on which packages are importable — a +`# type: ignore` that is **required** with `sentence-transformers` installed is +an **error** without it, and vice versa. Skip the install and your checker +grades different code than CI's, so a gate can red on lines you never touched. + +`ruff` is pinned to an exact version for the same reason: a newer ruff +reformats files that already passed, and the diff lands in your PR. + +## What surprises people + +These are the gates that fail most often. None of them are obvious from the +code alone. + +**Tests are type-checked, and they must be annotated.** `mypy` runs over +`tests/` as well as the package, in strict mode. A new test function without +annotations fails the gate: + +```python +async def test_thing(fixture): # fails +async def test_thing(fixture: Any) -> None: # passes +``` + +**Tests must assert something real.** A test whose body has no assertion, or +only a nullness check, is rejected — it passes whether or not the code works. +Assert on content: + +```python +assert result.status == 403 # good +assert result is not None # rejected: nullness only +gateway.check_origin(origin) # rejected: asserts nothing +``` + +If a test's point is that a call does *not* raise, pair it with the case that +does, so the test can actually fail: + +```python +gateway.check_origin("http://localhost:3000") # allowed +with pytest.raises(GatewayRejection): # ...and this still isn't + gateway.check_origin("https://evil.example.com") +``` + +**Formatting and lint have autofixes.** Run them rather than hand-fixing: + +```bash +python3 -m ruff check signalwire --fix +python3 -m ruff format signalwire +``` + +`__all__` must be sorted (`RUF022`) — append a name to the end and the gate +reds. The autofix handles it. + +**Docstrings have a floor.** Public symbols need one, measured against a +committed threshold. Adding public API without docstrings lowers coverage and +fails the gate. + +## Before you start + +**Check the open PRs.** More than one change here has been written twice +because two people fixed the same thing in the same week. + +**Wire shapes come from the engine, not from the docs.** If you are changing +what the SDK puts on the wire — an action shape, an enum value, a parameter +name — say in the PR where you confirmed it. The generated types under +`signalwire/signalwire/**/*_generated.py` are derived from the engine's own +schemas and are the closest authority in this repo. A change that contradicts +them needs a reason. + +## Opening the PR + +**Say if you change public API.** New or renamed public classes, methods, or +parameters have to be reflected in shared infrastructure that lives in a +private repo, and a maintainer lands that alongside your PR. You do not need +access to it — just flag it in the description so it does not get missed: + +> Changes public surface: adds `ChatGateway.router()`. + +**If you are contributing from a fork, CI will not run.** Not a mistake on +your part: pull requests from forks do not receive repository secrets, and one +of the setup steps needs one. Every job fails in seconds with +`Input required and not supplied: token`. + +Nothing you change will fix that. Run `scripts/run-ci.sh` locally, say in the +PR that you did, and a maintainer will re-run it from a branch in this repo. +Your commits keep your authorship. + +## Rules of engagement + +The engineering rules this project is held to — parity with the reference +implementation, what may and may not be excused, how tests are written — are +enforced by the gates, so `run-ci.sh` is the practical version of all of them. +Maintainers work from a fuller ruleset; you do not need it to contribute. From 30e9bf917b26c5d3e1e57b6b008e998d3733feea Mon Sep 17 00:00:00 2001 From: Anthony Minessale II Date: Thu, 13 Aug 2026 12:48:35 -0500 Subject: [PATCH 2/2] docs: move CONTRIBUTING under .github/ and excuse pytest.raises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gates caught this, both correctly. ROOT-HYGIENE keeps a public port root clear of anything not on its allowlist, and a new tracked root file is exactly what it exists to stop. `.github/` is where GitHub looks for CONTRIBUTING.md anyway — it surfaces identically from the PR and issue UI — so the file moves rather than the gate being widened. DOC-AUDIT resolves every symbol referenced in docs against the surface oracle, and the assertion example cites `pytest.raises`. Recorded in DOC_AUDIT_IGNORE.md as third-party, next to the other test-helper entries. Both were verified locally BEFORE this push and both passed — because the check ran before the files were staged, and both gates only see tracked files. Which is a fair demonstration of the guide's own first rule: run scripts/run-ci.sh, which stages nothing and reads the tree the way CI does. --- CONTRIBUTING.md => .github/CONTRIBUTING.md | 0 DOC_AUDIT_IGNORE.md | 4 ++++ 2 files changed, 4 insertions(+) rename CONTRIBUTING.md => .github/CONTRIBUTING.md (100%) diff --git a/CONTRIBUTING.md b/.github/CONTRIBUTING.md similarity index 100% rename from CONTRIBUTING.md rename to .github/CONTRIBUTING.md diff --git a/DOC_AUDIT_IGNORE.md b/DOC_AUDIT_IGNORE.md index 351441ab..23ba5097 100644 --- a/DOC_AUDIT_IGNORE.md +++ b/DOC_AUDIT_IGNORE.md @@ -81,6 +81,10 @@ LoggerFactory: structlog.stdlib.LoggerFactory — structlog include_router: FastAPI.include_router — framework method add_middleware: FastAPI.add_middleware — framework method +## pytest (third-party, used in CONTRIBUTING.md examples) + +raises: pytest.raises — third-party test helper shown in the contributing guide's assertion examples + ## prometheus_client (third-party) inc: prometheus_client.Counter.inc — monitoring example in search_deployment.md