From 90d7d2726832f760ec3539d1dd4edb433e96cbf6 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 30 Nov 2024 17:21:46 +0100 Subject: [PATCH 1/6] Add contributing guide --- CONTRIBUTING.md | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..a600aac --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,55 @@ +**Contributions are welcome.** + +## Contribution guidelines + +General [RIOT contribution guidelines](https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md) apply, +with some + +* focus points: + + * When wrapping a new RIOT feature, **keep it small**. + Attempting to implement all features of a particular RIOT subsystem easily gets caught in long iterations of review. + Try wrapping a minimal viable version first, and let's take it from there. + + * New features should come with tests, at least on a coarse granularity. + Tests in [`./tests/`](tests) are automatically picked up by CI. + +* and alterations: + + * RIOT has no coding conventions for Rust; instead, common Rust conventions apply. + Public items should be documented, + and code should be `cargo fmt` before each commit. + + * Merging criteria are not as strict as in RIOT OS: + RIOT maintainers may merge PRs without a 2nd pair of eyes + (but are encouraged to solicit review on larger changes). + + Before code from this crate gets used by RIOT OS, + it undergoes a secondary review step [similar to PR #20786](https://github.com/RIOT-OS/RIOT/pull/20786), + just like any external package code. + + * The license of this crate is "[MIT](https://spdx.org/licenses/MIT.html) OR [Apache-2.0](https://spdx.org/licenses/Apache-2.0.html)" as stated in [Cargo.toml](Cargo.toml) and the [README](README.md). + This eases interoperability with the rest of the Rust ecosystem; + beware that built binaries include RIOT OS, and thus have LGPL-2.1 components. + +## Common pitfalls + +* Threads and interrupts in RIOT are modelled as is [common](https://onevariable.com/blog/interrupts-is-threads/) in Rust: + Data needs to be [`Send`](https://doc.rust-lang.org/std/marker/trait.Send.html) to be passed around. + Whenever generic data (in particular references) is passed into C functions, + trait bounds need to ensure that it is `Send`. + +* Registering callbacks with RIOT OS typically means + that the OS may invoke the callback at any time. + Thus the callback either must be `'static`, + or it needs to be ensured that the callback is fully unregistered + before its lifetime ends. + + Note that we can not rely on [`Drop`](https://doc.rust-lang.org/std/ops/trait.Drop.html) to be called + (for any user may safely [`forget()`](https://doc.rust-lang.org/std/mem/fn.forget.html) an item at any time). + + Running user code in Rust closures is a common workaround + (eg. [in `thread::scope`](https://doc.riot-os.org/rustdoc/latest/riot_wrappers/thread/fn.scope.html)), + but beware that any items in that scope need to be [branded](http://plv.mpi-sws.org/rustbelt/ghostcell/paper.pdf) (like with the `'id` of the thread scope): + Otherwise, users might create nested scopes and switch around the provided types, + delaying their cleanup from the inner to the outer scope. From 830b3a5a36423e4b581e565482d3b2bca3135f65 Mon Sep 17 00:00:00 2001 From: chrysn Date: Mon, 9 Dec 2024 15:22:58 +0100 Subject: [PATCH 2/6] contributing: Point to Rust style guide --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a600aac..0f119d9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,8 @@ with some * and alterations: - * RIOT has no coding conventions for Rust; instead, common Rust conventions apply. + * RIOT has no coding conventions for Rust; + instead, [common Rust conventions](https://doc.rust-lang.org/stable/style-guide/) apply. Public items should be documented, and code should be `cargo fmt` before each commit. From 21ee7d07e68cb8ef0dabaa1b5d034ecb6d3f0635 Mon Sep 17 00:00:00 2001 From: chrysn Date: Mon, 9 Dec 2024 15:40:30 +0100 Subject: [PATCH 3/6] contributing: Mention draft PRs --- CONTRIBUTING.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f119d9..3326848 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,6 +11,10 @@ with some Attempting to implement all features of a particular RIOT subsystem easily gets caught in long iterations of review. Try wrapping a minimal viable version first, and let's take it from there. + * Let's **verify concepts early**. + Opening an issue before larger changes, creating them as draft PRs or discussing it on [our Matrix channel](https://matrix.to/#/#riot-os:matrix.org) + are all convenient ways to do that. + * New features should come with tests, at least on a coarse granularity. Tests in [`./tests/`](tests) are automatically picked up by CI. From ab88723172194084f40c6ad720e530286c27adf4 Mon Sep 17 00:00:00 2001 From: chrysn Date: Mon, 9 Dec 2024 15:41:53 +0100 Subject: [PATCH 4/6] contributing: Add tl;dr --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3326848..9096f6a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,7 @@ **Contributions are welcome.** +*tl;dr: Open issues. PR early, PR small. Tests are good. Callback lifetimes are hard.* + ## Contribution guidelines General [RIOT contribution guidelines](https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md) apply, From 3a37139441c47d6f53222a94688ae78a22590acd Mon Sep 17 00:00:00 2001 From: chrysn Date: Mon, 9 Dec 2024 15:50:34 +0100 Subject: [PATCH 5/6] README: link to contributing guide --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index dafdf61..5bbd4af 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,8 @@ and profit from better integration. Code conventions ---------------- +There is a [contributing guide](CONTRIBUTING.md) for this repository. + In older pieces of code (predating the use of C2Rust), static inline RIOT functions or expanded macros are used. To keep track of them, comments in the shape of ``EXPANDED ${FILE}:${LINE}`` are set (referring to line numbers in RIOT commit 6b96f69b). From 4d5eb3c243897c9d6f4ccb094bd3ff4005f0a701 Mon Sep 17 00:00:00 2001 From: chrysn Date: Mon, 9 Dec 2024 15:59:13 +0100 Subject: [PATCH 6/6] contributing: remind future editors of the intended audience --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9096f6a..154dbe9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,6 +2,8 @@ *tl;dr: Open issues. PR early, PR small. Tests are good. Callback lifetimes are hard.* + + ## Contribution guidelines General [RIOT contribution guidelines](https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md) apply,