Skip to content

Coding Guidelines

Felix Müller edited this page Nov 22, 2021 · 5 revisions

Since our team is continuously growing in size, the need arises to write down and discuss our coding guidelines.

Automation

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.

Official Guidelines and Best-Practices

Wherever automated tools cannot help (yet), we try to make use of existing guidelines and best-practices:

Git

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:
    1. A pull-request from your (feature-)branch to master.
    2. At least one approved review.
    3. Only 'gatekeepers' can then merge the approved PR (not sure this is still the case?).
  • Merging into master is done by merge squash, which is partially enforced by the repo settings and helps the git history to be 'clean'.

Branches

  • 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.

Formatting

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.

Formatting imports (use)

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.

CLion

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

VS Code

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+,) activate Editor: Format On Save (in JSON: "editor.formatOnSave")
  • cargo clippy warnings while coding: In the settings (Ctrl+,) set Rust-analyzer: Check On Save to clippy (in JSON: "rust-analyzer.checkOnSave.command": "clippy").

Cargo.toml style

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