/ 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