mirror of
https://gitlab.computer.surgery/matrix/grapevine.git
synced 2025-12-16 15:21:24 +01:00
add a style guide
This commit is contained in:
parent
42adad330e
commit
94c1a39437
2 changed files with 149 additions and 0 deletions
|
|
@ -6,3 +6,4 @@
|
||||||
* [Changelog](./changelog.md)
|
* [Changelog](./changelog.md)
|
||||||
* [Contributing](./contributing.md)
|
* [Contributing](./contributing.md)
|
||||||
* [Coordinated vulnerability disclosure](./contributing/security.md)
|
* [Coordinated vulnerability disclosure](./contributing/security.md)
|
||||||
|
* [Style guide](./contributing/style-guide.md)
|
||||||
|
|
|
||||||
148
book/contributing/style-guide.md
Normal file
148
book/contributing/style-guide.md
Normal file
|
|
@ -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.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue