-
Notifications
You must be signed in to change notification settings - Fork 47
Coding Guidelines
Since our team is continuously growing in size, the need arises to write down and discuss our coding guidelines.
We automate as much of our coding style as possible. rustfmt
takes care of the formatting and cargo clippy
does the static analysis (linter). Both tools need to be happy in order for your code changes to be accepted. This is enforced using CI on GitHub Actions.
Wherever automated tools cannot help (yet), we try to make use of existing guidelines and best-practices:
- https://rust-lang.github.io/api-guidelines/about.html
- https://rust-unofficial.github.io/patterns/intro.html
- https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/guide.md
There are a couple of conventions regarding our VCS (git). Some of them are enforced by the repository settings, some others are more soft:
- The stable branch is
master
- In order to merge into
master
, you need:- A pull-request from your (feature-)branch to
master
. - At least one approved review.
- Only 'gatekeepers' can then merge the approved PR (not sure this is still the case?).
- A pull-request from your (feature-)branch to
- Merging into master is done by
merge squash
, which is partially enforced by the repo settings and helps the git history to be 'clean'.
- Create branches for your work
- Keep the git history clean by rebasing your branches (rather than merging from
master
). -
Naming:
- Prefix the branch name with your initials (e.g.
fm-
), that way it is clear who is responsible for deleting the branch and who is allowed to force push to this branch. - Give your branch a descriptive name, separate words by either
-
or_
. - (Optional): Add the Github issue number as suffix, e.g.
-496
.
- Prefix the branch name with your initials (e.g.
Formatting is done using rustfmt
tool and the rustfmt.toml
config file in the root level. GitHub Actions will run rustfmt
and complain if any of your changes are not conformant with our current settings. So make sure you have your IDE setup to automatically run rstfmt
.
We try to let rustfmt
order our imports wherever possible. This means that we group all the use
statements together, without empty lines and let rustfmt
do its work. In the current setting, it will put local imports crate::
first and then order the rest of the imports alphabetically. There are some cases where we conditionally import crates, e.g.
#[cfg(all(not(feature = "std"), feature = "sgx"))]
use thiserror::Error;
These can and should be separate from the other imports. Also they should be placed before the regular imports. The reason being, that tools like CLion will auto-import crates and place them just after the last use
statement. If that last use
statement is the regular group, then everything works out fine. Otherwise we will end up with multiple groups of import statements.
If you're using CLion, you need to use these settings (Languages & Frameworks -> Rust -> Rustfmt):
- Use rustfmt instead of built-in formatter
- Run rustfmt on Save
If you're using VS Code: The official Rust extension is currently not to be recommended. Instead, use rust-analyzer. No matter which extension you choose to use: Make sure you've enabled only one of them. If both are used concurrently, they might mess with each other.
To use cargo fmt
& cargo clippy
automatically in VS Code (with rust-analyzer):
- Install rust-analyzer extension (Ctrl + shift + x)
-
cargo fmt
on save: In the settings (Ctrl+,) activateEditor: Format On Save
(in JSON:"editor.formatOnSave"
) -
cargo clippy
warnings while coding: In the settings (Ctrl+,) setRust-analyzer: Check On Save
toclippy
(in JSON:"rust-analyzer.checkOnSave.command": "clippy"
).
Unfortunately there is no formatting tool for the Cargo.toml
files (yet). So we have some conventions how to organize these:
- Use the one-liner import format:
frame-support = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "master", version = "4.0.0-dev" }
- Use
"
not'
for strings - Use alphabetical order within a grouping of dependencies