mirror of
https://gitlab.computer.surgery/matrix/grapevine.git
synced 2025-12-16 23:31:24 +01:00
148 lines
4.5 KiB
Markdown
148 lines
4.5 KiB
Markdown
# 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.
|