diff --git a/book/SUMMARY.md b/book/SUMMARY.md index 089a9ad7..cc933b8f 100644 --- a/book/SUMMARY.md +++ b/book/SUMMARY.md @@ -6,3 +6,4 @@ * [Changelog](./changelog.md) * [Contributing](./contributing.md) * [Coordinated vulnerability disclosure](./contributing/security.md) + * [Style guide](./contributing/style-guide.md) diff --git a/book/contributing/style-guide.md b/book/contributing/style-guide.md new file mode 100644 index 00000000..957a13a1 --- /dev/null +++ b/book/contributing/style-guide.md @@ -0,0 +1,148 @@ +# Style guide + +It is recommended that contributors follow this guide to minimize nitpicking on +their merge requests. + +However, this guide is not set in stone. It is open to changes as new patterns +emerge, requirements change, compelling arguments are made, and so on. The goal +is to document the existing style so it can be applied consistently, not to +ensure the style never changes. + +## Merge requests + +When updating a merge request branch, use `git rebase`; do not create merge +commits to merge other branches into a merge request branch. + +**Why?** This keeps the history simpler, and lacking merge commits makes it +easier to revert any individual commit. + +## Commit messages + +[Here's a good article][git-commit] on how to write good commit messages in +general. + +Specifically for this project: + +* Capitalizing the first letter is not required. +* It is recommended to avoid "conventional commits", as they take away from the + very limited subject line length, and will not be processed in an automated + fashion anyway. + +**Why?** The linked article explains why this is good practice. + +[git-commit]: https://cbea.ms/git-commit/ + +## Structuring commits + +Try to structure each commit so that it falls into one of the following +categories: + +* Refactoring, with no behavior change. +* Changing behavior, with no refactoring. +* Removing behavior, with no refactoring. +* Adding behavior, with no refactoring. +* Rewriting something completely. It is rare that these kinds of commits are + warranted. + +If you find yourself wanting to use the word "and" in the commit's subject line, +it should probably be broken into multiple commits. + +During code review, it's common to get feedback requesting changes to your +commits. To apply this feedback, do not make and push a new commit containing +the requested change. Instead, include the requested change in the commit of +yours that gave rise to the suggestion. If you are unfamiliar with rewriting +history in git, [this website][git-rebase] is a great tutorial. + +**Why?** Small, targeted, and well-explained commits make it easier for +reviewers to verify that a change has its intended effect. Or, for someone +running `git bisect` to find a more granular answer to why their test began +failing. + +[git-rebase]: https://git-rebase.io/ + +## `mod`/`use` order + +`mod` and `use` statements should appear in the following order, separated by +a blank line: + +1. `use` statements referring to `std`, `alloc`, or `core`, if any. +2. `use` statements referring to other crates, if any. +3. `use` statements referring to `super` or `crate`, if any. +4. Macro definitions that need to be accessible from child modules, if any. +5. `mod` statements, if any. +6. `use` statements referring to modules declared by the above `mod` statements, + if any. + +`rust-analyzer` and `rustfmt` automate most of this except points 4 and 5. + +**Why?** Consistency is good. + +## Testing + +When writing tests, be sure to keep the contents of [this article][cargo-test] +in mind. Especially, keeping Cargo unit tests in a dedicated tests file +(mentioned towards the end of the article). + +**Why?** The linked article explains why this is good practice. + +[cargo-test]: https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html + +## Tracing + +`tracing` events should: + +1. Start with a capital letter (when applicable). + +`tracing` events should not: + +1. End with punctuation. +2. Interpolate values into the event's message. + * Instead, add those values as structured fields. + +**Why?** Consistency is good. Also, interpolating values into the event message +essentially defeats the point of structured logging. + +### Examples + +#### 1 + +```rust,ignore +// This does not conform because it does not start with a capital letter. +info!("started pentametric fan"); + +// Do this instead: +info!("Started pentametric fan"); +``` + +#### 2 + +```rust,ignore +// This does not conform because it ends with punctuation. +info!("Started pentametric fan."); + +// Do this instead: +info!("Started pentametric fan"); +``` + +#### 3 + +```rust,ignore +// This does not conform because it interpolates values into the event's +// message. +warn!("Noticed {} discombobulated waneshafts", count); + +// Do this instead: +warn!(count, "Noticed discombobulated waneshafts"); +``` + +## Services + +Services are abstraction units that live inside the `src/service` directory. + +Calling service constructors must not cause side effects, with a few exceptions: + +* Database reads. +* Local filesystem reads. + +**Why?** This restriction makes it possible to implement subcommands that run +"offline" that reuse service code.