/ CONTRIBUTING.md
CONTRIBUTING.md
1 # CONTRIBUTING 2 3 Contributions are very welcome. 4 5 ## Contributing Issues 6 7 Contributing issues are an excellent way to ensure your problem will 8 eventually get looked at. 9 10 If creating an issue, please make sure to include: 11 - actual behaviour you're observing, 12 - the behaviour you were expecting instead, 13 - steps to reproduce your issue, 14 - the output of `rad debug`, 15 - the contents of the log files referred to by that output. 16 17 18 ## Contributing Code 19 20 When contributing code, please follow these 21 simple guidelines. 22 23 * Follow the coding guidelines when proposing code changes ([see below](#code-style)). 24 * Write properly formatted git commits messages ([see below](#writing-git-commit-messages)). 25 * Read the DCO and if you are contributing a significant amount of code, make sure your commits are signed off, using `git commit -s`. 26 * Contribute code via a Radicle Patch, by following the [guide][]. 27 * If it's a feature addition or major change, check with the maintainers first 28 before submitting a patch. We wouldn't want you to waste your time! 29 * If you need help or would like to discuss your changes, come to our community chat on [Zulip][zulip]. 30 31 [guide]: https://radicle.xyz/guides/user#working-with-patches 32 [zulip]: https://radicle.zulipchat.com 33 34 ### Submitting patches 35 36 Patch formatting follows the same rules as commit formatting. See below. 37 38 ### Linting & formatting 39 40 Always check your code with the linter (`clippy`), by running: 41 42 $ cargo clippy --workspace --tests 43 44 And make sure your code is formatted with, using: 45 46 $ cargo fmt 47 48 Finally, ensure there is no trailing whitespace anywhere. 49 50 ### Running tests 51 52 Make sure all tests are passing with: 53 54 $ cargo test --workspace 55 56 Some tests require `jq`. If `jq` is not detected, these tests will succeed 57 without effectively testing anything. 58 59 ### Checking the docs 60 61 If you make documentation changes, you may want to check whether there are any 62 warnings or errors: 63 64 $ cargo doc --workspace --all-features 65 66 ### Code style 67 68 The following code guidelines will help make code review smoother. 69 70 #### Use of `unwrap` and `expect` 71 72 Use `unwrap` only in either of three circumstances: 73 74 1. Based on manual static analysis, you've concluded that it's impossible for 75 the code to panic; so unwrapping is *safe*. An example would be: 76 77 let list = vec![a, b, c]; 78 let first = list.first().unwrap(); 79 80 2. The panic caused by `unwrap` would indicate a bug in the software, and it 81 would be impossible to continue in that case. 82 83 3. The `unwrap` is part of test code, ie. `cfg!(test)` is `true`. 84 85 In the first and second case, document `unwrap` call sites with a comment prefixed 86 with `SAFETY:` that explains why it's safe to unwrap, eg. 87 88 // SAFETY: Node IDs are valid ref strings. 89 let r = RefString::try_from(node.to_string()).unwrap(); 90 91 Use `expect` only if the function expects certain invariants that were not met, 92 either due to bad inputs, or a problem with the environment; and include the 93 expectation in the message. For example: 94 95 logger::init(log::Level::Debug) 96 .expect("logger must only be initialized once"); 97 98 #### Module imports 99 100 Modules are declared at the top of the file, before the imports. Public modules 101 are separated from private modules with a blank line: 102 103 mod git; 104 mod storage; 105 106 pub mod refs; 107 108 use std::time; 109 use std::process; 110 111 ... 112 113 Imports are organized in groups, from least specific to more specific: 114 115 use std::collections::HashMap; // First, `std` imports. 116 use std::process; 117 use std::time; 118 119 use git_ref_format as format; // Then, external dependencies. 120 use serde_json::Value; 121 122 use crate::crypto::PublicKey; // Finally, local crate imports. 123 use crate::storage::refs::Refs; 124 use crate::storage::RemoteId; 125 126 #### Variable naming 127 128 Use short 1-letter names when the variable scope is only a few lines, or the context is 129 obvious, eg. 130 131 if let Some(e) = result.err() { 132 ... 133 } 134 135 Use 1-word names for function parameters or variables that have larger scopes: 136 137 pub fn commit(repo: &Repository, sig: &Signature) -> Result<Commit, Error> { 138 ... 139 } 140 141 Use the most descriptive names for globals: 142 143 pub const KEEP_ALIVE_DELTA: LocalDuration = LocalDuration::from_secs(30); 144 145 #### Function naming 146 147 Stay concise. Use the function doc comment to describe 148 what the function does, not the name. Keep in mind functions are in the 149 context of the parent module and/or object and repeating that would be 150 redundant. 151 152 #### Logging 153 154 When writing log statements, always include a `target` and include enough 155 context in the log message that it is useful on its own, eg. 156 157 debug!(target: "service", "Routing table updated for {rid} with seed {nid}"); 158 159 Check the file you are working on for what the `target` name should be; most 160 logs should be at the *debug* level. 161 162 ### Dependencies 163 164 Before adding any code dependencies, check with the maintainers if this is okay. 165 In general, we try not to add external dependencies unless it's necessary. 166 Dependencies increase counter-party risk, build-time, attack surface, and 167 make code harder to audit. 168 169 ### Documentation 170 171 Public types and functions should be documented. Modules *may* be documented, 172 if you see the need. 173 174 Code comments should usually be full english sentences, and add missing context 175 for the reader: 176 177 // Ensure that our inventory is recorded in our routing table, and we are tracking 178 // all of it. It can happen that inventory is not properly tracked if for eg. the 179 // user creates a new repository while the node is stopped. 180 for rid in self.storage.inventory()? { 181 ... 182 183 ### Referring to radicle.xyz in Code 184 185 While <https://radicle.xyz> is the main website of the project, and also the domain 186 associated with COBs implemented in this repo, we strive to write code that is as 187 independent as reasonably possible from this particular domain name. For example, it 188 should not be used for default configuration values, or if it is, there should be a 189 way to override. 190 191 This makes it easier to re-package Radicle for distribution under a different domain 192 or fork it altogether. It also tends to produce better, more flexible, code. 193 194 In tests, instead use names that are compliant with RFC 2606, e.g. 195 "radicle.example.com". 196 197 Note that as of 2025-08, there are still a few mentions of "radicle.xyz" in the 198 codebase (mostly tests or user hints, fallback for configuration), and some of them 199 are not easy to remove. However, this is in no way a justification to add more 200 references. 201 202 ### Proposing changes 203 204 When proposing changes via a patch: 205 206 * Isolate changes in separate commits to make the review process easier. 207 * Don't make unrelated changes, unless it happens to be an obvious improvement to 208 code you are touching anyway ("boyscout rule"). 209 * Rebase on `master` when needed. 210 * Keep your changesets small, specific and uncontroversial, so that they can be 211 merged more quickly. 212 * If the change is substantial or requires re-architecting certain parts of the 213 codebase, write a proposal in English first, and get consensus on that before 214 proposing the code changes. 215 216 **Preparing commits** 217 218 1. Each commit in your patch must pass all the tests, lints and checks. This is 219 so that they can be built into binaries and to make git bisecting possible. 220 2. Do not include any commits that are fixes or refactorings of previous patch 221 commits. These should be squashed to the minimal diff required to make the 222 change, unless it's helpful to make a large change over multiple commits, 223 while still respecting (1). Do not include `fixup!` commits either. 224 3. A commit *may* include a category prefix such as `cli:` or `node:` if it 225 mainly concerns a certain area of the codebase. For example. These prefixes 226 should usually be the name of the crate, minus any common prefix. Eg. 227 `cli:`, and *not* `radicle-cli:`. For documentation, you can use `docs:`, 228 and for CI-related files, you can use `ci:`. 229 230 To help with the above, use `git commit --amend` and `git rebase -i`. You can 231 also interactively construct a commit from a working tree using `git add -p`. 232 233 ### Writing commit messages 234 235 A properly formed git commit subject line should always be able to complete the 236 following sentence: 237 238 If applied, this commit will _____ 239 240 In addition, it should be capitalized and *must not* include a period. 241 242 For example, the following message is well formed: 243 244 Add support for .gif files 245 246 While these ones are **not**: `Adding support for .gif files`, 247 `Added support for .gif files`, `add support for .gif files`. 248 249 When it comes to formatting, here's a model git commit message[1]: 250 251 Capitalized, short (50 chars or less) summary 252 253 More detailed explanatory text, if necessary. Wrap it to about 72 254 characters or so. In some contexts, the first line is treated as the 255 subject of an email and the rest of the text as the body. The blank 256 line separating the summary from the body is critical (unless you omit 257 the body entirely); tools like rebase can get confused if you run the 258 two together. 259 260 Write your commit message in the imperative: "Fix bug" and not "Fixed bug" 261 or "Fixes bug." This convention matches up with commit messages generated 262 by commands like git merge and git revert. 263 264 Further paragraphs come after blank lines. 265 266 - Bullet points are okay, too. 267 268 - Typically a hyphen or asterisk is used for the bullet, followed by a 269 single space, with blank lines in between, but conventions vary here. 270 271 - Use a hanging indent. 272 273 --- 274 275 [1]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html