From 22ec8fc77c721977deed4a1ae18b68fb0c3067be Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 5 Sep 2026 05:49:37 +0000 Subject: [PATCH 01/10] CONTRIBUTING.md: state every contribution requirement here Twelve requirements exist today in AGENTS.md with no counterpart anywhere in CONTRIBUTING.md, so they bind only the contributors who read the agent file: real-time cycle-time rule, never trust values from remote clients, never hand-edit generated files, never reformat libs/, regenerate docs/JSON-RPC.md after an RPC change, the priority order, AI disclosure, shellcheck + shfmt, QT_VERSION_CHECK guards, filling in the PR template, no reformatting of untouched code, and saying so when something was not verified. The duplication has already drifted: AGENTS.md asks every contribution for test evidence, CONTRIBUTING.md asked it of AI contributions only. That sentence is now unconditional here. Wording is kept close to the AGENTS.md source so this reviews as a move, not as twelve new rules. Four posting expectations from docs/agents/COMMENTING.md move in the same way, as "Commenting and reviewing". A fifth, on honouring a stated engagement preference, is reworded so that disagreeing once is explicitly welcome, and moves here because it then applies to everyone rather than to agents. Two paragraphs are not moves and should be read as additions: wire compatibility (do not renumber PROTMESSID_*, retired IDs stay reserved as the OLD entries in src/protocol.h) and the note that the three clang-format lists have to be updated together. Each states a rule the project already enforces but no file records. AGENTS.md and docs/agents/COMMENTING.md are reduced to pointers in the next commit; this one must land first so nothing points at a section that does not exist yet. CHANGELOG: SKIP --- CONTRIBUTING.md | 53 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa7c326d55..070e83e817 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,16 +5,34 @@ Weโ€™d really appreciate your support! Please ensure that you understand the fol - If a [Github issue](https://github.com/jamulussoftware/jamulus/issues) for your feature/bug fix already exists, write a message in that issue indicating that you want to work on it. - Otherwise, please [post on the GitHub Discussions](https://github.com/jamulussoftware/jamulus/discussions) and say that you are planning to do some coding and explain why. Then we can discuss the specification. -- Please begin coding only after we have agreed on a specification to avoid putting a lot of effort into something that may not be accepted later. +- Please begin coding only after we have agreed on a specification to avoid putting a lot of effort into something that may not be accepted later. Changes to networking or the wire protocol, to threading, or to the build system always need this discussion first. -- Keep commits and Pull Requests focussed on one logical change only. In case you implement multiple features, open multiple smaller PRs instead of large one. Large PRs may become stale since they are not reviewable and be closed after a long time of inactivity. +- Keep commits and Pull Requests focussed on one logical change only, and make the smallest change that does it. Do not mix refactoring with a fix or a feature, and do not reformat untouched code: both hide the change under review. In case you implement multiple features, open multiple smaller PRs instead of large one. Large PRs may become stale since they are not reviewable and be closed after a long time of inactivity. ## Jamulus project/source code general principles +Where these principles pull against each other, resolve the conflict in this order: **stability > low latency and real-time safety > backwards compatibility > maintainability > new features.** This order settles conflicts only - new features are welcome. + ### 1. Stability Instabilities during live performances such as WorldJam are not acceptable. As a result, stability has been, and must continue to be the most important requirement. The following principles are designed to support this. +#### Real-time safety + +Do not introduce code that prevents processing of audio within the _minimum_ cycle time for _any_ frame (i.e. worst case must remain viable); DO test this and produce evidence to support the change. + +- This covers sound processing in `src/sound`, network processing in `src/socket.cpp` and mixing in `src/server.cpp`. +- Potential problems include (but are not limited to): memory allocation, file I/O, locks. +- Where possible, move processing off the real-time thread with queued signals. + +#### Input arriving over the network + +Do not trust values sent by remote clients, servers or directories. Validate the size and bounds of everything read from the network before it reaches an array index, a length calculation or an allocation. Malformed input is how a crash gets into a release; see [SECURITY.md](SECURITY.md) for reporting one you find in a released version. + +#### Wire compatibility + +Clients and servers of different versions have to keep understanding each other, so do not renumber `PROTMESSID_*` and do not change the layout of a protocol message that already exists. Retired message IDs stay reserved - see the `OLD` entries in `src/protocol.h`. Extend the protocol by adding a new message ID. + ### 2. [Keep it Simple and Stupid](https://en.wikipedia.org/wiki/KISS_principle) and 3. [Do One Thing and Do It Well](https://en.wikipedia.org/wiki/Unix_philosophy#Do_One_Thing_and_Do_It_Well) If a feature or function can be accomplished in another way by another system or method, it is preferable not to build that feature into Jamulus. Rather than implementing each and every feature as part of Jamulus, we concentrate on a stable core and implement interfaces for interaction with third-party components as needed. The [JSON-RPC](https://github.com/jamulussoftware/jamulus/blob/main/docs/JSON-RPC.md) API for example, allows you to communicate with the client and server from outside the application. @@ -32,6 +50,8 @@ There are several ways to run clang-format: - By hand: run `clang-format -i ` +Adding a source directory or a new file extension? `make clang_format` and the CI check read separate lists, and the [workflow's own comment](.github/workflows/coding-style-check.yml) says to update all three together: its `extensions:` list, its `paths:` filter, and `CLANG_FORMAT_SOURCES` in `Jamulus.pro`. + ##### Style definition Please see the [.clang_format file](https://github.com/jamulussoftware/jamulus/blob/main/.clang-format) in the root folder. In summary: @@ -47,10 +67,22 @@ Please install and use [pylint](https://pylint.org/) to scan any Python code. There is a configuration file that defines some overrides, and note the [Editorconfig file](.editorconfig) in the project too. +#### Shell scripts +Our CI runs [shellcheck](https://www.shellcheck.net/) and `shfmt` on `.sh` files. Please run both before committing. + +#### Files not to edit by hand + +- Generated sources - `moc_*.cpp`, `ui_*.h`, `qrc_*.cpp` and `*.qm` - are build products. Change what they are generated from and regenerate. +- Third-party code under `libs/` keeps its upstream formatting. Do not reformat it, and keep any change to it to the minimum needed. +- `docs/JSON-RPC.md` is generated. If you change a JSON-RPC method (for example in `src/clientrpc.cpp` or `src/serverrpc.cpp`), regenerate the document with `tools/generate_json_rpc_docs.py` in the same Pull Request - our CI fails otherwise. +- The `ChangeLog` file - see [Documentation/Acknowledgements](#documentationacknowledgements) below. + ### Using AI - When you use AI as part of your work, remember that it is a tool that you choose to use and your code will be judged in the same way as any other submission. However, we encourage you to share your AI experiences, as it is an emerging technology, by highlighting how you used AI and give your own review of its performance. +Please disclose AI-generated text at the end of the comment, issue or Pull Request description that carries it, for example `> ๐Ÿค– Used AI: , `. Do not put such notes in code comments. + AI-assisted contributions **must** follow the same standards as every other contribution. The submitter remains the author and is expected to understand and stand behind every submitted line. [AGENTS.md](AGENTS.md) is the entry point into this repository for AI Agents. Low-effort contributions might be closed without comment. ### Licensing @@ -84,7 +116,8 @@ _While Android and iOS aren't officially supported, please don't break their bui Please try to avoid breaking any build by introducing platform-specific code. Check the Github builds all worked before raising a pull request. Check to see if any newly introduced Qt calls, parameters, properties or constants are available in the minimum supported Qt version, which is currently **5.12.2**. Note that code _style_ in a file may be Qt 4.x. While you should normally stick to existing style, if you make large-scale modifications, updating to Qt 5.12.2 style is recommended. -Maintain C++11 compatibility throughout the code. +Guard any call that needs a newer Qt with `#if QT_VERSION >= QT_VERSION_CHECK(...)`. +Maintain C++11 compatibility throughout the code (the Android build uses C++17 for Oboe). ### Dependencies @@ -103,9 +136,13 @@ We're using git to develop Jamulus. To contribute, you should get familiar to gi Have a look at our [guide for translators](docs/TRANSLATING.md) - especially read the git related part. If you need more in depth information, the [git-scm book](https://git-scm.com/book/en/v2) might also help you getting started. If you have any questions, don't hesitate to ask, as git can be very confusing. +Please fill in the [Pull Request template](.github/pull_request_template.md) - it is the checklist reviewers work from. + +Some build targets (iOS, Windows JACK, Linux armhf/arm64) are skipped by default. If your change touches one of them, add the `AUTOBUILD` tag described under [Dependencies](#dependencies) to the Pull Request description. + ### Testing -To check that there are no errors, please perform an appropriate local (build/feature/typo) test depending on what you did. Briefly explain in the PR what and how you tested your work and attach evidence - logs, screenshots, small testing scripts or similar depending on your change. +To check that there are no errors, please perform an appropriate local (build/feature/typo) test depending on what you did. Briefly explain in the PR what and how you tested your work and attach evidence - logs, screenshots, small testing scripts or similar depending on your change. If you did not run or verify part of your change, say so rather than leaving it implied. Keep an eye on the CI checks for quality or compile issues after opening a pull request and fix them as needed. You can also test the build on your repository by naming your branch `autobuild/` which will start the building process on your repo. ### Ownership @@ -115,6 +152,14 @@ The submitter of an Issue or a Pull Request is responsible for its care and feed Authors are strongly encouraged to update their initial posts/PR descriptions or title to reflect the current state of play, amends, enhancements, outstanding issues, etc., to reduce effort for others in understanding a PR or an Issue. Admins reserve the right to do this as they see fit. +### Commenting and reviewing + +- Test what you can test before you claim it - a build, a log, a run - and cut what you cannot. Words like *presumably*, *should* and *likely* usually mark a sentence that needs a measurement, or needs deleting. +- Comment when you add evidence or an answer the thread does not have yet, in the shortest form that carries it. Let an exchange between others finish, and re-read the thread just before posting - it may have moved while you were writing. +- If a comment turns out to be wrong or incomplete, edit it so that the error leaves the page. Further evidence about the same finding belongs in that comment rather than in a new one. +- Open an issue for a defect you can reproduce, and put the reproduction in the body. +- If someone states how they want to be engaged on a thread - for example, no AI-written replies - follow it while it stands. Disagreeing is welcome: say so once, with your reason; a preference can rest on a misunderstanding on either side. + ### Documentation/Acknowledgements The ChangeLog must be updated for each new feature or bug fix. Please include a single-sentence suggestion for that as part of your pull request description after the `CHANGELOG: ` keyword. Do not modify the ChangeLog file as part of your PR as it will lead to conflicts. From 9f4d4bca9e9f0e961f076a71c928c5434e8628cc Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 5 Sep 2026 05:49:37 +0000 Subject: [PATCH 02/10] AGENTS.md: point at CONTRIBUTING.md instead of restating it Every requirement AGENTS.md carried is now stated in CONTRIBUTING.md, so this file keeps only what is not a requirement - where things are, how to build, how to run - plus one link per rule. A second copy drifts, and the drift had already happened: this file demanded test evidence of every contribution while CONTRIBUTING.md asked it of AI contributions only. All 16 CONTRIBUTING.md anchors linked from here resolve against the file as of the previous commit. docs/agents/COMMENTING.md gets the same treatment and keeps the one line that would be wrong to ask of a human contributor: leave the starting of new discussions to humans. CHANGELOG: SKIP --- AGENTS.md | 81 +++++++++++++-------------------------- docs/agents/COMMENTING.md | 11 +++--- 2 files changed, 33 insertions(+), 59 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c24cb4a290..2f95431263 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,9 +2,9 @@ Real-time networked music jamming app. Qt/C++ qmake project. Client and server share one codebase; entry point: `src/main.cpp`. Qt project configuration in `Jamulus.pro`. -**Make the smallest possible change. One logical change per PR. Never mix refactoring with fixes/features.** +**[`CONTRIBUTING.md`](CONTRIBUTING.md) is the source of truth for everything this project requires of a contribution; this file does not restate those requirements, and defers to it if the two ever disagree.** Read it before changing code, and before opening or commenting on an issue, Pull Request or discussion here. -Priority order: Stability > Low latency / real-time safety > Backwards compatibility > Maintainability > New features. This order resolves conflicts only โ€” new features are welcome. +What is below is orientation only: where things are, and how to build and run them. --- @@ -12,60 +12,33 @@ Priority order: Stability > Low latency / real-time safety > Backwards compatibi **Before running a build**, read `COMPILING.md` for your compile target. It includes build commands, platform-specific dependencies and `CONFIG` flags. `.github/autobuild` contains the build scripts for the GitHub Actions workflow. Read these files if you are stuck and need an example. -**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`, enabled with `--jsonrpcport` and `--jsonrpcsecretfile`) where possible. Connecting a client needs a build without `serveronly` (`COMPILING.md`, "Compile time arguments"); `serveronly` rejects `-c`. State what you tested in the PR with evidence. GitHub Actions builds multiple platforms โ€” on failure read the failing step's log. - -## Never Do - -**`Never Do` rules are absolute** - -- Introduce code that prevents processing of audio within the minimum cycle time for any frame (i.e. worst case must remain viable); DO test this and produce evidence to support the change - - this covers sound process in `src/sound`, network processing in `src/socket.cpp` and mixing in `src/server.cpp` - - potential problems include (but not limited to): memory allocation, file I/O, locks - - where possible, move processing off the real-time thread with queued signals -- Trust values from remote clients โ€” validate size/bounds on all network input (malformed input crashes). -- Edit generated files (`moc_*.cpp`, `ui_*.h`, `qrc_*.cpp`, `*.qm`) โ€” regenerate; don't edit/reformat third-party code in `libs/`. -- Edit `ChangeLog` directly โ€” use a `CHANGELOG:` line in the PR. - -## Always - -- Attach test evidence (logs/output) to the PR โ€” never just assert something works. -- Say so if you did not run or verify something. - -## Ask first - -- Architecture changes (networking/protocol, threading, build system) โ€” open an issue to discuss (see `CONTRIBUTING.md`). - -## Qt / portability - -- Minimum Qt: **5.12.2**. Qt 6 recommended (iOS: Qt 5.15+ required, Qt 6 iOS buggy). Guard newer APIs with `#if QT_VERSION >= QT_VERSION_CHECK(...)`. -- C++11 (C++17 on Android for Oboe). -- Preserve platform support. -- Desktop: Windows 10+, macOS 10.10+, Ubuntu 20.04+/Debian 11+. - -## Style (C / C++ / Obj-C++) - -- **CI uses clang-format** (version in `.github/workflows/coding-style-check.yml`). -- Run `make clang_format` before committing (works only after qmake). -- CI runs **shellcheck + shfmt** on `.sh` files; **pylint** (config: `.pylintrc`) on `.py` files in `tools/`. -- New contributions: AGPL 3.0+ license header. Pre-3.12.1dev code: GPL 3.0+ (see `CONTRIBUTING.md`). -- Use `tr ( "Hello %1" ).arg ( name )` for user-facing strings โ€” never string concatenation. - -## JSON-RPC - -- Changing RPC methods (e.g. `src/clientrpc.cpp` / `src/serverrpc.cpp`) requires regenerating `docs/JSON-RPC.md` with `tools/generate_json_rpc_docs.py` (CI fails otherwise). -- Requires `--jsonrpcport` + `--jsonrpcsecretfile` at runtime. Binds to localhost by default. Secret requires โ‰ฅ16 characters. - -## PR expectations - -- No unrelated cleanup or reformatting of untouched code. Discuss features in an issue before implementing. -- Branch names starting with `autobuild` trigger CI builds on your fork. -- Follow `.github/pull_request_template.md`. Include `CHANGELOG:` line. Add `AUTOBUILD: Please build all targets` for skipped targets (iOS, Windows JACK, Linux armhf/arm64) if touched; see `.github/workflows/autobuild.yml`. -- Self reviewed against "Priority order" above? -- Disclose AI-generated text at the end of Comments/PRs. (e.g: `> ๐Ÿค– Used AI: , `) โ€” never in code comments. +## Run it + +A plain build gives one binary that is both client and server. Run the server headless with `-s -n`; connect a client with `-n -c localhost` (on Linux this may need jackd โ€” `jackd -d dummy`). A `CONFIG+=serveronly` binary rejects `-c`. Drive it through the JSON-RPC API where that is possible: it needs `--jsonrpcport` and `--jsonrpcsecretfile`; see `docs/JSON-RPC.md`. GitHub Actions builds several platforms; on failure, read the failing step's log. + +## Where the rules are + +| Before youโ€ฆ | Read | +|---|---| +| start writing anything at all | [the opening bullets](CONTRIBUTING.md#contributing-to-jamulus) | +| resolve a design tradeoff | [general principles](CONTRIBUTING.md#jamulus-projectsource-code-general-principles) | +| touch `src/sound`, `src/socket.cpp` or `src/server.cpp` | [Real-time safety](CONTRIBUTING.md#real-time-safety) | +| parse anything that arrived over the network | [Input arriving over the network](CONTRIBUTING.md#input-arriving-over-the-network) | +| change an existing protocol message | [Wire compatibility](CONTRIBUTING.md#wire-compatibility) | +| format code | [Source code consistency](CONTRIBUTING.md#source-code-consistency) | +| edit a generated file or `libs/` | [Files not to edit by hand](CONTRIBUTING.md#files-not-to-edit-by-hand) | +| use AI for any part of the work | [Using AI](CONTRIBUTING.md#using-ai) | +| add a file, or copy code in | [Licensing](CONTRIBUTING.md#licensing) | +| use a Qt or C++ feature that may be too new | [Supported platforms](CONTRIBUTING.md#supported-platforms) | +| add a dependency | [Dependencies](CONTRIBUTING.md#dependencies) | +| write user-facing text | [User experience](CONTRIBUTING.md#user-experience) | +| open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership) | +| post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and `docs/agents/COMMENTING.md` | +| write a `CHANGELOG:` line | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | ## Read when relevant -- `CONTRIBUTING.md` โ€” process, style, licensing + - `COMPILING.md` โ€” full build per platform, CONFIG flags table - `docs/JAMULUS_PROTOCOL.md` โ€” network protocol, packet IDs, ack rules -- `docs/agents/COMMENTING.md` โ€” rules when commenting on GitHub +- `docs/agents/COMMENTING.md` โ€” commenting on GitHub - `SECURITY.md` โ€” security reporting diff --git a/docs/agents/COMMENTING.md b/docs/agents/COMMENTING.md index 75ba5aa138..b65ca8b76d 100644 --- a/docs/agents/COMMENTING.md +++ b/docs/agents/COMMENTING.md @@ -1,7 +1,8 @@ ## Posting (comments, issues, discussions) -- Test any claim you can test before posting it โ€” a build, a log, a run โ€” and cut what you can't; *presumably*, *should*, and *likely* mark sentences that need a measurement or deletion. -- Comment only when you add evidence or an answer the thread lacks, in the shortest form that carries it; wait out an active human exchange, re-read the thread just before posting โ€” it may have moved while you drafted โ€” and treat a maintainer's stated preference about engagement as the rule. -- If a posted comment proves wrong or incomplete, edit it in place so the error leaves the page; new evidence on the same finding also belongs in the existing comment, not a new one. -- Open a new issue only for a defect you can reproduce, with the reproduction in the body; open a new PR only after a maintainer has agreed the change is wanted; leave starting discussions to humans. -- Disclose AI-generated text โ€” the form is in `AGENTS.md`, PR expectations. +What is expected of anyone posting here is in [CONTRIBUTING.md ยง Commenting and reviewing](../../CONTRIBUTING.md#commenting-and-reviewing) and [Using AI](../../CONTRIBUTING.md#using-ai); it applies to an agent exactly as it applies to a person, so it is not repeated here. + +Two things are specific to an agent: + +- Leave the starting of new discussions to humans. +- Open a new PR only after a maintainer has agreed the change is wanted. From 0e6fa6107e9fc7399e6938ffd05552aab9a69e39 Mon Sep 17 00:00:00 2001 From: jrd Date: Fri, 11 Sep 2026 21:43:54 +0000 Subject: [PATCH 03/10] AGENTS.md: keep the always-on rules on top, and one table instead of two indexes Both from review on #3946. The smallest-change rule and the priority order fire on every decision, so there is no action to gate them behind: they belong in the always-loaded file. 42 words. The Read when relevant list was a second index of the same tree - COMPILING.md and docs/agents/COMMENTING.md were each named twice in this file - so its entries become rows and every row now names the action that triggers the read. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QCBQnyakc16a1nr69kCy91 --- AGENTS.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2f95431263..2b3e4ea47b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,10 @@ Real-time networked music jamming app. Qt/C++ qmake project. Client and server s **[`CONTRIBUTING.md`](CONTRIBUTING.md) is the source of truth for everything this project requires of a contribution; this file does not restate those requirements, and defers to it if the two ever disagree.** Read it before changing code, and before opening or commenting on an issue, Pull Request or discussion here. +**Make the smallest possible change. One logical change per PR. Never mix refactoring with fixes/features.** + +Priority order: Stability > Low latency / real-time safety > Backwards compatibility > Maintainability > New features. This order resolves conflicts only โ€” new features are welcome. + What is below is orientation only: where things are, and how to build and run them. --- @@ -35,10 +39,6 @@ A plain build gives one binary that is both client and server. Run the server he | open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership) | | post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and `docs/agents/COMMENTING.md` | | write a `CHANGELOG:` line | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | - -## Read when relevant - -- `COMPILING.md` โ€” full build per platform, CONFIG flags table -- `docs/JAMULUS_PROTOCOL.md` โ€” network protocol, packet IDs, ack rules -- `docs/agents/COMMENTING.md` โ€” commenting on GitHub -- `SECURITY.md` โ€” security reporting +| build for a platform other than the two above | [`COMPILING.md`](COMPILING.md) | +| change how clients, servers and directories talk to each other | [`docs/JAMULUS_PROTOCOL.md`](docs/JAMULUS_PROTOCOL.md) | +| report a security vulnerability โ€” never as an issue | [`SECURITY.md`](SECURITY.md) | From ed5178d1fa0e73b84f4d10d1391603432f496152 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 12 Sep 2026 16:34:55 +0000 Subject: [PATCH 04/10] Apply two review suggestions from #3946 "copy code in" reads as copying into the repo, which is only half of what the Licensing section governs; "from elsewhere" names the source, which is what decides whether a licence question exists at all. The priority order's "new features are welcome" stood without the qualifier the next section supplies. Item 2 is the one that says a feature is often better built outside Jamulus, so a reader who stops at the order gets the opposite impression of the one the two paragraphs together give. Both lines are pljones's wording, taken verbatim. AGENTS.md keeps its own copy of the order without the pointer: it has no item 2 to point at. CHANGELOG: SKIP Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MEXBpjaa8oA5PPgQcZg5Sq --- AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2b3e4ea47b..4ed2ab74f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ A plain build gives one binary that is both client and server. Run the server he | format code | [Source code consistency](CONTRIBUTING.md#source-code-consistency) | | edit a generated file or `libs/` | [Files not to edit by hand](CONTRIBUTING.md#files-not-to-edit-by-hand) | | use AI for any part of the work | [Using AI](CONTRIBUTING.md#using-ai) | -| add a file, or copy code in | [Licensing](CONTRIBUTING.md#licensing) | +| add a file, or copy code from elsewhere | [Licensing](CONTRIBUTING.md#licensing) | | use a Qt or C++ feature that may be too new | [Supported platforms](CONTRIBUTING.md#supported-platforms) | | add a dependency | [Dependencies](CONTRIBUTING.md#dependencies) | | write user-facing text | [User experience](CONTRIBUTING.md#user-experience) | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 070e83e817..a2796b78db 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,7 +11,7 @@ Weโ€™d really appreciate your support! Please ensure that you understand the fol ## Jamulus project/source code general principles -Where these principles pull against each other, resolve the conflict in this order: **stability > low latency and real-time safety > backwards compatibility > maintainability > new features.** This order settles conflicts only - new features are welcome. +Where these principles pull against each other, resolve the conflict in this order: **stability > low latency and real-time safety > backwards compatibility > maintainability > new features.** This order settles conflicts only - new features are welcome (but read item 2 below first). ### 1. Stability From 4e53c5385334701ed966191fab0838627ffe78ef Mon Sep 17 00:00:00 2001 From: jrd Date: Mon, 14 Sep 2026 14:06:03 +0000 Subject: [PATCH 05/10] AGENTS.md: leave the Build and Testing section to #3944 #3944 rewrites the same six lines: the Linux and macOS build paragraphs become a pointer to COMPILING.md, and the section is retitled "Build and Test". Editing that block here collided with it in one hunk, in either merge direction. Nothing in that block is a contribution requirement, which is what this PR is about, so it is dropped rather than resolved: main's Testing paragraph is restored byte for byte, and the table row that read "a platform other than the two above" now reads "any platform", since the two lines it pointed at are the ones #3944 removes. The two branches now merge in either order. CHANGELOG: SKIP Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RjKL2JBt9AcyfCVS5Gbwpc --- AGENTS.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4ed2ab74f6..33f2f4eed2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,9 +16,7 @@ What is below is orientation only: where things are, and how to build and run th **Before running a build**, read `COMPILING.md` for your compile target. It includes build commands, platform-specific dependencies and `CONFIG` flags. `.github/autobuild` contains the build scripts for the GitHub Actions workflow. Read these files if you are stuck and need an example. -## Run it - -A plain build gives one binary that is both client and server. Run the server headless with `-s -n`; connect a client with `-n -c localhost` (on Linux this may need jackd โ€” `jackd -d dummy`). A `CONFIG+=serveronly` binary rejects `-c`. Drive it through the JSON-RPC API where that is possible: it needs `--jsonrpcport` and `--jsonrpcsecretfile`; see `docs/JSON-RPC.md`. GitHub Actions builds several platforms; on failure, read the failing step's log. +**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`, enabled with `--jsonrpcport` and `--jsonrpcsecretfile`) where possible. Connecting a client needs a build without `serveronly` (`COMPILING.md`, "Compile time arguments"); `serveronly` rejects `-c`. State what you tested in the PR with evidence. GitHub Actions builds multiple platforms โ€” on failure read the failing step's log. ## Where the rules are @@ -39,6 +37,6 @@ A plain build gives one binary that is both client and server. Run the server he | open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership) | | post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and `docs/agents/COMMENTING.md` | | write a `CHANGELOG:` line | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | -| build for a platform other than the two above | [`COMPILING.md`](COMPILING.md) | +| build for any platform | [`COMPILING.md`](COMPILING.md) | | change how clients, servers and directories talk to each other | [`docs/JAMULUS_PROTOCOL.md`](docs/JAMULUS_PROTOCOL.md) | | report a security vulnerability โ€” never as an issue | [`SECURITY.md`](SECURITY.md) | From 1d103de5ef76ce5e549d20e28316d40868209049 Mon Sep 17 00:00:00 2001 From: jrd Date: Tue, 15 Sep 2026 11:56:28 +0000 Subject: [PATCH 06/10] CONTRIBUTING.md: a build is not a test @pljones on #3946: "If building on multiple platforms matters, so does testing - but build is not testing. (If it were, there would be zero runtime bugs...)", with a pattern for non-GUI changes. It lands in the Testing section, which is where this PR collects contribution requirements. Commands stay out of it: AGENTS.md carries those, and COMPILING.md the per-platform build flags. What the JSON-RPC API can drive was measured against main rather than read off the method list: a headless Server and a headless Client over a dummy JACK driver, with setName, setSkillLevel, setInstrumentCode, setMuted, setFaderLevel and sendChatText issued on the Client's RPC port and read back from the Server's own getClients and its chatMessageReceived notification. Connecting is not part of it - jamulusclient/connect does not exist (-32601), so a Client is still put in front of a Server with -c. CHANGELOG: SKIP Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a2796b78db..ebd8c55657 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,6 +145,8 @@ Some build targets (iOS, Windows JACK, Linux armhf/arm64) are skipped by default To check that there are no errors, please perform an appropriate local (build/feature/typo) test depending on what you did. Briefly explain in the PR what and how you tested your work and attach evidence - logs, screenshots, small testing scripts or similar depending on your change. If you did not run or verify part of your change, say so rather than leaving it implied. Keep an eye on the CI checks for quality or compile issues after opening a pull request and fix them as needed. You can also test the build on your repository by naming your branch `autobuild/` which will start the building process on your repo. +For a change that is not confined to the GUI, name the platforms it affects and make sure the GitHub build covers them on your branch. A build is not a test, though: exercise the change itself on both `main` and your branch and report the difference in the PR rather than asserting it. A headless Server and a headless Client on one machine need no audio hardware - [AGENTS.md](AGENTS.md) has the commands - and the [JSON-RPC API](docs/JSON-RPC.md) drives the session once the Client has connected: names, skill levels, instruments, mute state, fader levels and chat text are all settable from outside, and the Server reports what it sees. + ### Ownership The submitter of an Issue or a Pull Request is responsible for its care and feeding (this also holds for contributions that were assisted by AI), answering all questions directed at them, and making agreed changes if necessary. In case you use AI and do not understand some outputs, clarify before submission if possible (for example, by asking the project team or an AI agent) or else clearly state this. From d904a4044c4a4943594f7115daea3b353d3da583 Mon Sep 17 00:00:00 2001 From: jrd Date: Wed, 16 Sep 2026 05:36:25 +0000 Subject: [PATCH 07/10] AGENTS.md: the don'ts first, the AI disclosure form back, and the build line out of Testing @pljones on #3946, three points, all in the files he read them in. "There's a few bits here that end up as 'Don't do this' - it's probably worth grouping them, just after 'start writing anything at all'." Which rows those are is not a guess: main's own AGENTS.md carries a "## Never Do" section, and this PR folded it into the table. Its four bullets -- real-time safety, network input, generated files and libs/, ChangeLog -- plus the security row are now the five rows directly under the first one. Two are reworded to read as the prohibitions they are: the generated-file row, and the ChangeLog row, which was phrased as "write a CHANGELOG: line" and hid main's actual rule that ChangeLog is not edited by hand. "GitHub Actions builds multiple platforms - on failure read the failing step's log. This isn't testing." It was inside the Testing paragraph; it is now the last sentence of the build paragraph above, where it describes a build. The Testing paragraph ends by pointing at CONTRIBUTING.md's Testing section instead, which 1d103de5 already gave the non-GUI pattern he asked for. This edit was dropped once before, in 4e53c538, because it collided with #3944 in one hunk; #3944 merged 2026-09-14, so the collision is gone. @ann0see on the same PR: "I think this MUST remain in AGENTS.md only. Here we just have a disclosure sentence, in AGENTS.md we have the exact example." Done as stated, and the example restored to AGENTS.md is the project's own merged wording from 7849b502, not a new one. CONTRIBUTING.md keeps the requirement -- which is @pljones's case on the same thread, text drafted by AI and submitted by someone who never opens AGENTS.md -- and now points at AGENTS.md for the form. Every one of the 19 CONTRIBUTING.md anchors referenced from AGENTS.md resolves against the heading set at this head; no relative link in either file is dead. CHANGELOG: SKIP Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 14 ++++++++------ CONTRIBUTING.md | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 33f2f4eed2..e087316f26 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,27 +8,31 @@ Real-time networked music jamming app. Qt/C++ qmake project. Client and server s Priority order: Stability > Low latency / real-time safety > Backwards compatibility > Maintainability > New features. This order resolves conflicts only โ€” new features are welcome. +**AI disclosure, the exact form:** `> ๐Ÿค– Used AI: , `, at the end of the comment, issue or Pull Request description it belongs to โ€” never in a code comment. The requirement itself is [Using AI](CONTRIBUTING.md#using-ai). + What is below is orientation only: where things are, and how to build and run them. --- ## Build and Test -**Before running a build**, read `COMPILING.md` for your compile target. It includes build commands, platform-specific dependencies and `CONFIG` flags. `.github/autobuild` contains the build scripts for the GitHub Actions workflow. Read these files if you are stuck and need an example. +**Before running a build**, read `COMPILING.md` for your compile target. It includes build commands, platform-specific dependencies and `CONFIG` flags. `.github/autobuild` contains the build scripts for the GitHub Actions workflow. Read these files if you are stuck and need an example. GitHub Actions builds multiple platforms โ€” on failure read the failing step's log. -**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`, enabled with `--jsonrpcport` and `--jsonrpcsecretfile`) where possible. Connecting a client needs a build without `serveronly` (`COMPILING.md`, "Compile time arguments"); `serveronly` rejects `-c`. State what you tested in the PR with evidence. GitHub Actions builds multiple platforms โ€” on failure read the failing step's log. +**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`, enabled with `--jsonrpcport` and `--jsonrpcsecretfile`) where possible. Connecting a client needs a build without `serveronly` (`COMPILING.md`, "Compile time arguments"); `serveronly` rejects `-c`. State what you tested in the PR with evidence. A build is not a test: [Testing](CONTRIBUTING.md#testing) says what to exercise and what to report. ## Where the rules are | Before youโ€ฆ | Read | |---|---| | start writing anything at all | [the opening bullets](CONTRIBUTING.md#contributing-to-jamulus) | -| resolve a design tradeoff | [general principles](CONTRIBUTING.md#jamulus-projectsource-code-general-principles) | | touch `src/sound`, `src/socket.cpp` or `src/server.cpp` | [Real-time safety](CONTRIBUTING.md#real-time-safety) | | parse anything that arrived over the network | [Input arriving over the network](CONTRIBUTING.md#input-arriving-over-the-network) | +| edit a generated file or `libs/` โ€” don't, by hand | [Files not to edit by hand](CONTRIBUTING.md#files-not-to-edit-by-hand) | +| edit `ChangeLog` directly โ€” don't; use a `CHANGELOG:` line in the PR | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | +| report a security vulnerability โ€” never as an issue | [`SECURITY.md`](SECURITY.md) | +| resolve a design tradeoff | [general principles](CONTRIBUTING.md#jamulus-projectsource-code-general-principles) | | change an existing protocol message | [Wire compatibility](CONTRIBUTING.md#wire-compatibility) | | format code | [Source code consistency](CONTRIBUTING.md#source-code-consistency) | -| edit a generated file or `libs/` | [Files not to edit by hand](CONTRIBUTING.md#files-not-to-edit-by-hand) | | use AI for any part of the work | [Using AI](CONTRIBUTING.md#using-ai) | | add a file, or copy code from elsewhere | [Licensing](CONTRIBUTING.md#licensing) | | use a Qt or C++ feature that may be too new | [Supported platforms](CONTRIBUTING.md#supported-platforms) | @@ -36,7 +40,5 @@ What is below is orientation only: where things are, and how to build and run th | write user-facing text | [User experience](CONTRIBUTING.md#user-experience) | | open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership) | | post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and `docs/agents/COMMENTING.md` | -| write a `CHANGELOG:` line | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | | build for any platform | [`COMPILING.md`](COMPILING.md) | | change how clients, servers and directories talk to each other | [`docs/JAMULUS_PROTOCOL.md`](docs/JAMULUS_PROTOCOL.md) | -| report a security vulnerability โ€” never as an issue | [`SECURITY.md`](SECURITY.md) | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ebd8c55657..15054b0e62 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ Our CI runs [shellcheck](https://www.shellcheck.net/) and `shfmt` on `.sh` files - When you use AI as part of your work, remember that it is a tool that you choose to use and your code will be judged in the same way as any other submission. However, we encourage you to share your AI experiences, as it is an emerging technology, by highlighting how you used AI and give your own review of its performance. -Please disclose AI-generated text at the end of the comment, issue or Pull Request description that carries it, for example `> ๐Ÿค– Used AI: , `. Do not put such notes in code comments. +Please disclose AI-generated text at the end of the comment, issue or Pull Request description that carries it. Do not put such notes in code comments. [AGENTS.md](AGENTS.md) carries the exact form. AI-assisted contributions **must** follow the same standards as every other contribution. The submitter remains the author and is expected to understand and stand behind every submitted line. [AGENTS.md](AGENTS.md) is the entry point into this repository for AI Agents. Low-effort contributions might be closed without comment. From 4087b10e6906e1d38ed498ef0d3eb7aa1a544fb0 Mon Sep 17 00:00:00 2001 From: jrd Date: Wed, 16 Sep 2026 15:40:04 +0000 Subject: [PATCH 08/10] Address the two outside-diff review findings, and one they implied @coderabbitai raised both in review bodies rather than inline, so neither created a thread. AGENTS.md: docs/agents/COMMENTING.md was cited only from the "post a comment or a review" row, yet the rule it carries that gates opening a PR -- "Open a new PR only after a maintainer has agreed the change is wanted" -- is a PR rule. Anyone reading the table to open a PR never reached it. Now cited from both rows. Adding a second target beside CONTRIBUTING.md is the pattern the comment row already used, and @ann0see asked on this PR that agent-specific material stay in AGENTS.md. Both citations are now links. The comment row had it as bare backticked text while every other cell in the table is a link. CONTRIBUTING.md: the bullet telling everyone to open an issue for any reproducible defect had no security exception, in the file this PR makes responsible for carrying every requirement. SECURITY.md is unambiguous -- "Please do not open GitHub issues for security vulnerabilities" -- so the bullet now says so and points there. The same sentence's neighbour in this PR, the network-input paragraph, scoped SECURITY.md to "a released version". SECURITY.md sets no such limit, and two different scopings of one security rule in one file is the inconsistency this PR exists to remove. Dropped. Checked: every relative link in both files resolves on disk, and all 17 CONTRIBUTING.md anchors referenced from AGENTS.md match a heading. CHANGELOG: SKIP Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 4 ++-- CONTRIBUTING.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e087316f26..c0ee96c724 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,7 @@ What is below is orientation only: where things are, and how to build and run th | use a Qt or C++ feature that may be too new | [Supported platforms](CONTRIBUTING.md#supported-platforms) | | add a dependency | [Dependencies](CONTRIBUTING.md#dependencies) | | write user-facing text | [User experience](CONTRIBUTING.md#user-experience) | -| open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership) | -| post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and `docs/agents/COMMENTING.md` | +| open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership), and [`docs/agents/COMMENTING.md`](docs/agents/COMMENTING.md) | +| post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and [`docs/agents/COMMENTING.md`](docs/agents/COMMENTING.md) | | build for any platform | [`COMPILING.md`](COMPILING.md) | | change how clients, servers and directories talk to each other | [`docs/JAMULUS_PROTOCOL.md`](docs/JAMULUS_PROTOCOL.md) | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 15054b0e62..b0d8c6bccf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,7 @@ Do not introduce code that prevents processing of audio within the _minimum_ cyc #### Input arriving over the network -Do not trust values sent by remote clients, servers or directories. Validate the size and bounds of everything read from the network before it reaches an array index, a length calculation or an allocation. Malformed input is how a crash gets into a release; see [SECURITY.md](SECURITY.md) for reporting one you find in a released version. +Do not trust values sent by remote clients, servers or directories. Validate the size and bounds of everything read from the network before it reaches an array index, a length calculation or an allocation. Malformed input is how a crash gets into a release; see [SECURITY.md](SECURITY.md) for how to report one you find. #### Wire compatibility @@ -159,7 +159,7 @@ Admins reserve the right to do this as they see fit. - Test what you can test before you claim it - a build, a log, a run - and cut what you cannot. Words like *presumably*, *should* and *likely* usually mark a sentence that needs a measurement, or needs deleting. - Comment when you add evidence or an answer the thread does not have yet, in the shortest form that carries it. Let an exchange between others finish, and re-read the thread just before posting - it may have moved while you were writing. - If a comment turns out to be wrong or incomplete, edit it so that the error leaves the page. Further evidence about the same finding belongs in that comment rather than in a new one. -- Open an issue for a defect you can reproduce, and put the reproduction in the body. +- Open an issue for a defect you can reproduce, and put the reproduction in the body. Never open one for a security vulnerability โ€” [SECURITY.md](SECURITY.md) says how to report those. - If someone states how they want to be engaged on a thread - for example, no AI-written replies - follow it while it stands. Disagreeing is welcome: say so once, with your reason; a preference can rest on a misunderstanding on either side. ### Documentation/Acknowledgements From dfe3ba82b517d50330f19c4e6ee5b6f0c6e8edbf Mon Sep 17 00:00:00 2001 From: jrd Date: Wed, 16 Sep 2026 17:25:28 +0000 Subject: [PATCH 09/10] Fold in /code-review and /simplify: stop restating, just point Five review passes over the previous two commits. What they agreed on is that several edits answered the question inside the pointer instead of pointing, which is the habit this PR exists to break. AGENTS.md:5 claimed CONTRIBUTING.md is the source of truth for "everything" and that this file "does not restate those requirements". That is falsifiable two lines down -- the smallest-change rule and the priority order are both restatements, inherited from main. Patching it with "the one exception is the AI disclosure form" made it worse: an exception list that was already wrong when written. The tiebreak was the load-bearing half and it stands alone. AI disclosure: AGENTS.md now carries the example string and nothing else, which is what @ann0see asked for, and CONTRIBUTING.md keeps the requirement. Previously both files spelled out placement and the code-comment prohibition, and each pointed at the other. "Exact form" became "example form" in both files: main writes it as "(e.g: ...)" and @ann0see's words were "the exact example", so mandating the literal string would invent a requirement no maintainer set. Table rows route instead of answering. The generated-file row keeps a broad trigger so a functional change under libs/ still matches it -- CONTRIBUTING.md:76 permits those and forbids reformatting, so the row cannot carry a flat prohibition. The ChangeLog row drops the mechanism it had copied out of CONTRIBUTING.md, keeping the direct link that saves a hop. Testing loses "State what you tested in the PR with evidence", a third copy of CONTRIBUTING.md:145. Both SECURITY.md references stop paraphrasing what SECURITY.md says. One of them had already drifted into claiming a "released version" scope that SECURITY.md does not set -- one rule stored twice, diverging on its first edit. Checked after: 0 dead relative links, 17 CONTRIBUTING.md anchors all resolving, 20 table rows with equal field counts, 0 em dashes in a file that uses spaced hyphens. Not done here, and worth raising separately: .github/ISSUE_TEMPLATE/ config.yml has no security contact_link, so nothing intercepts someone clicking "New issue" with a vulnerability. That is the surface GitHub provides for it, and no sentence in CONTRIBUTING.md reaches that reader. CHANGELOG: SKIP Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 10 +++++----- CONTRIBUTING.md | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c0ee96c724..a009e5747f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,13 +2,13 @@ Real-time networked music jamming app. Qt/C++ qmake project. Client and server share one codebase; entry point: `src/main.cpp`. Qt project configuration in `Jamulus.pro`. -**[`CONTRIBUTING.md`](CONTRIBUTING.md) is the source of truth for everything this project requires of a contribution; this file does not restate those requirements, and defers to it if the two ever disagree.** Read it before changing code, and before opening or commenting on an issue, Pull Request or discussion here. +**[`CONTRIBUTING.md`](CONTRIBUTING.md) is the source of truth for what this project requires of a contribution; where this file and CONTRIBUTING.md disagree, CONTRIBUTING.md wins.** Read it before changing code, and before opening or commenting on an issue, Pull Request or discussion here. **Make the smallest possible change. One logical change per PR. Never mix refactoring with fixes/features.** Priority order: Stability > Low latency / real-time safety > Backwards compatibility > Maintainability > New features. This order resolves conflicts only โ€” new features are welcome. -**AI disclosure, the exact form:** `> ๐Ÿค– Used AI: , `, at the end of the comment, issue or Pull Request description it belongs to โ€” never in a code comment. The requirement itself is [Using AI](CONTRIBUTING.md#using-ai). +**AI disclosure, the example form:** `> ๐Ÿค– Used AI: , `. The requirement itself is [Using AI](CONTRIBUTING.md#using-ai). What is below is orientation only: where things are, and how to build and run them. @@ -18,7 +18,7 @@ What is below is orientation only: where things are, and how to build and run th **Before running a build**, read `COMPILING.md` for your compile target. It includes build commands, platform-specific dependencies and `CONFIG` flags. `.github/autobuild` contains the build scripts for the GitHub Actions workflow. Read these files if you are stuck and need an example. GitHub Actions builds multiple platforms โ€” on failure read the failing step's log. -**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`, enabled with `--jsonrpcport` and `--jsonrpcsecretfile`) where possible. Connecting a client needs a build without `serveronly` (`COMPILING.md`, "Compile time arguments"); `serveronly` rejects `-c`. State what you tested in the PR with evidence. A build is not a test: [Testing](CONTRIBUTING.md#testing) says what to exercise and what to report. +**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`, enabled with `--jsonrpcport` and `--jsonrpcsecretfile`) where possible. Connecting a client needs a build without `serveronly` (`COMPILING.md`, "Compile time arguments"); `serveronly` rejects `-c`. A build is not a test: [Testing](CONTRIBUTING.md#testing) says what to exercise and what to report. ## Where the rules are @@ -27,8 +27,8 @@ What is below is orientation only: where things are, and how to build and run th | start writing anything at all | [the opening bullets](CONTRIBUTING.md#contributing-to-jamulus) | | touch `src/sound`, `src/socket.cpp` or `src/server.cpp` | [Real-time safety](CONTRIBUTING.md#real-time-safety) | | parse anything that arrived over the network | [Input arriving over the network](CONTRIBUTING.md#input-arriving-over-the-network) | -| edit a generated file or `libs/` โ€” don't, by hand | [Files not to edit by hand](CONTRIBUTING.md#files-not-to-edit-by-hand) | -| edit `ChangeLog` directly โ€” don't; use a `CHANGELOG:` line in the PR | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | +| edit a generated file, or anything under `libs/` | [Files not to edit by hand](CONTRIBUTING.md#files-not-to-edit-by-hand) | +| update the `ChangeLog` | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) | | report a security vulnerability โ€” never as an issue | [`SECURITY.md`](SECURITY.md) | | resolve a design tradeoff | [general principles](CONTRIBUTING.md#jamulus-projectsource-code-general-principles) | | change an existing protocol message | [Wire compatibility](CONTRIBUTING.md#wire-compatibility) | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b0d8c6bccf..55619b0e34 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,7 @@ Do not introduce code that prevents processing of audio within the _minimum_ cyc #### Input arriving over the network -Do not trust values sent by remote clients, servers or directories. Validate the size and bounds of everything read from the network before it reaches an array index, a length calculation or an allocation. Malformed input is how a crash gets into a release; see [SECURITY.md](SECURITY.md) for how to report one you find. +Do not trust values sent by remote clients, servers or directories. Validate the size and bounds of everything read from the network before it reaches an array index, a length calculation or an allocation. Malformed input is how a crash gets into a release; see [SECURITY.md](SECURITY.md). #### Wire compatibility @@ -81,7 +81,7 @@ Our CI runs [shellcheck](https://www.shellcheck.net/) and `shfmt` on `.sh` files - When you use AI as part of your work, remember that it is a tool that you choose to use and your code will be judged in the same way as any other submission. However, we encourage you to share your AI experiences, as it is an emerging technology, by highlighting how you used AI and give your own review of its performance. -Please disclose AI-generated text at the end of the comment, issue or Pull Request description that carries it. Do not put such notes in code comments. [AGENTS.md](AGENTS.md) carries the exact form. +Please disclose AI-generated text at the end of the comment, issue or Pull Request description that carries it. Do not put such notes in code comments. [AGENTS.md](AGENTS.md) carries the example form. AI-assisted contributions **must** follow the same standards as every other contribution. The submitter remains the author and is expected to understand and stand behind every submitted line. [AGENTS.md](AGENTS.md) is the entry point into this repository for AI Agents. Low-effort contributions might be closed without comment. @@ -159,7 +159,7 @@ Admins reserve the right to do this as they see fit. - Test what you can test before you claim it - a build, a log, a run - and cut what you cannot. Words like *presumably*, *should* and *likely* usually mark a sentence that needs a measurement, or needs deleting. - Comment when you add evidence or an answer the thread does not have yet, in the shortest form that carries it. Let an exchange between others finish, and re-read the thread just before posting - it may have moved while you were writing. - If a comment turns out to be wrong or incomplete, edit it so that the error leaves the page. Further evidence about the same finding belongs in that comment rather than in a new one. -- Open an issue for a defect you can reproduce, and put the reproduction in the body. Never open one for a security vulnerability โ€” [SECURITY.md](SECURITY.md) says how to report those. +- Open an issue for a defect you can reproduce, and put the reproduction in the body. Never open one for a security vulnerability - follow [SECURITY.md](SECURITY.md). - If someone states how they want to be engaged on a thread - for example, no AI-written replies - follow it while it stands. Disagreeing is welcome: say so once, with your reason; a preference can rest on a misunderstanding on either side. ### Documentation/Acknowledgements From 302a27809ebbeb7b848bea8437013d39511a9759 Mon Sep 17 00:00:00 2001 From: jrd Date: Wed, 16 Sep 2026 23:00:04 +0000 Subject: [PATCH 10/10] Apply pljones's two CONTRIBUTING.md suggestions from #3946 Both from his 2026-09-16 review. :76 - his suggested wording for the libs/ rule, and his follow-up asking for it to live under a heading of its own rather than in the files-not-to-edit list. New `#### Third-party Libraries` section carries his text; checked character-identical to the suggestion once the list marker is dropped and double spaces collapse to the file's single-space style. It adds two requirements the file did not have: a libs/ change MUST be agreed first, and MUST go upstream after acceptance. A pointer bullet stays behind in the list. That is the one thing he did not ask for, and it is the same shape as the pre-existing ChangeLog bullet two lines below it ("The `ChangeLog` file - see [...] below."), so it follows the file's own convention rather than inventing one. The list is a lookup table; someone about to run a formatter over libs/ looks there, not under a heading named for the libraries. :139 - his suggested wording for the PR-template line, with `See also see` reduced to `See also`. Otherwise byte-identical to his suggestion. Anchors checked mechanically, not by eye: all 26 headings slugged, every in-page link resolved, no duplicate slugs. #third-party-libraries and #documentationacknowledgements both land. /code-review and /simplify both run over the diff. Nothing applied from either. Skipped, with reasons: the AUTOBUILD finding is wrong (the template already says to uncomment the tag on the line above it); two "already the state" false positives (line 143 is already two sentences; the bullet already carries the no-reformat rule inline); and the placement question - the new section's MUST-agree/MUST-submit rules are governance sitting under a style-scoped parent - is real, raised by two reviewers independently, and left alone deliberately: pljones named this heading, two maintainers are mid-review, and it is a design question for them, not a commit. Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 55619b0e34..5989339abf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,10 +73,14 @@ Our CI runs [shellcheck](https://www.shellcheck.net/) and `shfmt` on `.sh` files #### Files not to edit by hand - Generated sources - `moc_*.cpp`, `ui_*.h`, `qrc_*.cpp` and `*.qm` - are build products. Change what they are generated from and regenerate. -- Third-party code under `libs/` keeps its upstream formatting. Do not reformat it, and keep any change to it to the minimum needed. +- Third-party code under `libs/` - do not reformat it, and see [Third-party Libraries](#third-party-libraries) below before changing it. - `docs/JSON-RPC.md` is generated. If you change a JSON-RPC method (for example in `src/clientrpc.cpp` or `src/serverrpc.cpp`), regenerate the document with `tools/generate_json_rpc_docs.py` in the same Pull Request - our CI fails otherwise. - The `ChangeLog` file - see [Documentation/Acknowledgements](#documentationacknowledgements) below. +#### Third-party Libraries + +Third-party code under `libs/` keeps its upstream formatting. Do not reformat it. Where a change to it is required, this MUST be agreed first and kept to the minimum needed. The change MUST be submitted upstream, after acceptance into Jamulus. + ### Using AI - When you use AI as part of your work, remember that it is a tool that you choose to use and your code will be judged in the same way as any other submission. However, we encourage you to share your AI experiences, as it is an emerging technology, by highlighting how you used AI and give your own review of its performance. @@ -136,7 +140,7 @@ We're using git to develop Jamulus. To contribute, you should get familiar to gi Have a look at our [guide for translators](docs/TRANSLATING.md) - especially read the git related part. If you need more in depth information, the [git-scm book](https://git-scm.com/book/en/v2) might also help you getting started. If you have any questions, don't hesitate to ask, as git can be very confusing. -Please fill in the [Pull Request template](.github/pull_request_template.md) - it is the checklist reviewers work from. +Please fill in the [Pull Request template](.github/pull_request_template.md) without removing any sections - it is the checklist reviewers work from. See also [Documentation/Acknowledgements](#documentationacknowledgements) for the `CHANGELOG:` keyword. Some build targets (iOS, Windows JACK, Linux armhf/arm64) are skipped by default. If your change touches one of them, add the `AUTOBUILD` tag described under [Dependencies](#dependencies) to the Pull Request description.