diff --git a/docs/STYLE_GUIDE.md b/docs/STYLE_GUIDE.md index 84e5126ea..085fecad4 100644 --- a/docs/STYLE_GUIDE.md +++ b/docs/STYLE_GUIDE.md @@ -3,22 +3,21 @@ As a basis, the [Substrate Style Guide](https://docs.substrate.io/build/troubleshoot-your-code/) should be taken into account. In addition to that, the following sections -further elaborate the style guide used in this repository. +further elaborate on the style guide used in this repository. ## Comments - Comments **must** be wrapped at 100 chars per line. -- Comments **must** be formulated in markdown. -## Doc comments +## Comments and Docstrings - Documentation is written using Markdown syntax. - Function documentation should be kept lean and mean. Try to avoid documenting the parameters, and instead choose self-documenting parameter names. If parameters interact in a complex manner (for example, if two arguments of type `Vec` must have the same length), then add a paragraph explaining this. -- Begin every docstring with a meaningful one sentence description of the - function in third person. +- Begin every docstring with a meaningful one-sentence description of the + function in the third person. - Avoid WET documentation such as this: ```rust @@ -45,8 +44,9 @@ further elaborate the style guide used in this repository. (to make understanding the benchmarks easier). - Docstrings for dispatchables need not document the `origin` parameter, but should specify what origins the dispatchable may be called by. -- Docstrings for dispatchables **must** include the events that the dispatchable - emits. +- Docstrings for dispatchables **must** include the high-level events that the + dispatchable emits and state under which conditions these events are emitted. +- Document side-effects of functions. - Use `#![doc = include_str!("../README.md")]`. - Detail non-trivial algorithms in comments inside the function. @@ -58,6 +58,10 @@ An example of a good docstring would be this: /// /// May only be (successfully) called by `RejectOrigin`. The fraction of the advisory bond that is /// slashed is determined by `AdvisoryBondSlashPercentage`. +/// +/// # Emits +/// +/// - `MarketRejected` on success. pub fn reject_market( origin: OriginFor, #[pallet::compact] market_id: MarketIdOf, @@ -76,32 +80,29 @@ duplicating documentation. - Format code contained in macro invocations (`impl_benchmarks!`, `decl_runtime_apis!`, homebrew macros in `runtime/`, etc.) and attributes (`#[pallet::weight(...)`, etc.) manually. -- Add trailing commas in macro invocations manually, as rustfmt won't add them - automatically. - - ```rust - ensure!( - a_very_very_very_very_very_very_very_long_variable, - b_very_very_very_very_very_very_very_long_variable, // This comma is not ensured by rustfmt. - ) - ``` ## Code Style - Never use panickers. -- Prefer double turbofish `Vec::::new()` over single turbofish - `>::new()`. - All branches of match expressions **should** be explicit. Avoid using the catch-all `_ =>`. -- When changing enums, maintain the existing order and add variants only at the - end of the enum to prevent messing up indices. -- Maintain lexicographical ordering of traits in `#[derive(...)]` attributes. +- When removing variants from enums that are used in storage or emitted, then + explicitly state the scale index of each variant: + ```rust + enum E { + #[codec(index = 0)] + V1, + #[codec(index = 1)] + V2, + /// --- snip! --- + } + ``` ## Crate and Pallet Structure -- Don't dump all code into `lib.rs`. Split code multiple files (`types.rs`, +- Don't dump all code into `lib.rs`. Split code into multiple files (`types.rs`, `traits.rs`, etc.) or even modules (`types/`, `traits/`, etc.). -- Changes to pallets **must** retain order of dispatchables. +- Changes to pallets **must** retain the order of dispatchables. - Sort pallet contents in the following order: - `Config` trait - Type values @@ -113,4 +114,45 @@ duplicating documentation. - Hooks - Dispatchables - Pallet's public and private functions - - Trait impelmentations + - Trait implementations + +## Code Style and Design + +- Exceed 70 lines of code per function only in exceptional circumstances. Aim + for less. +- No `while` in production. All `for` loops must have a maximum number of + passes. +- Use depth checks when using recursion in production. Use recursion only if the + algorithm is defined using recursion. +- Avoid `mut` in production code if possible without much pain. +- Mark all extrinsics `transactional`, even if they satisfy + the verify-first/write-later principle. +- Avoid indentation over five levels; never go over seven levels. +- All public functions must be documented. Documentation of `pub(crate)` and + private functions is optional but encouraged. +- Keep modules lean. Only exceed 1,000 lines of code per file in exceptional + circumstances. Aim for less (except in `lib.rs`). Consider splitting modules + into separate files. Auto-generated files are excluded. + +## Workflow + +- Merges require one review. Additional reviews may be requested. +- Every merge into a feature branch requires a review. +- Aim for at most 500 LOC added per PR. Only exceed 1,000 LOC lines added in a + PR in exceptional circumstances. Plan ahead and break a large PR into smaller + PRs targeting a feature branch. Feature branches are exempt from this rule. +- Reviews take priority over most other tasks. +- Reviewing a PR should not take longer than two business days. Aim for shorter + PRs if the changes are complex. +- A PR should not be in flight (going from first `s:ready-for-review` to + `s:accepted`) for longer than two weeks. Aim for shorter PRs if the changes + are complex. + +## Testing + +- Aim for 100% code coverage, excluding only logic errors that are raised on + inconsistent state. In other words: All execution paths **should** be tested. + There should be a clear justification for every LOC without test coverage. +- For larger modules, use one test file per extrinsic for unit tests. Make unit + tests as decoupled as possible from other modules. Place end-to-end and + integration tests in extra files. diff --git a/docs/review_checklist.md b/docs/review_checklist.md index 90c94b2ce..3b9220c61 100644 --- a/docs/review_checklist.md +++ b/docs/review_checklist.md @@ -53,6 +53,7 @@ - [ ] The try-runtime passes without any warnings (substrate storage operations often just log a warning instead of failing, so these warnings usually point to problem which could break the storage). +- [ ] Ensure that the [STYLE_GUIDE] is observed. ## Events @@ -85,3 +86,4 @@ Additional info (similar to the remark emitted by anywhere in the chain storage. [docs.zeitgeist.pm]: docs.zeitgeist.pm +[STYLE_GUIDE]: ./STYLE_GUIDE.md