Format code w/ `cargo fmt` and add formatting check to CI
Closed this issue · 3 comments
Sometimes in previous PR's, code that has differed from the Rust style guidelines has had to be manually pointed out in review. Unfortunately, using cargo fmt
ends up causing a large number of changes, because this isn't already enforced.
I'd like to propose that we begin checking that PR's follow formatting guidelines, by adding a check in CI.
Pros:
- Contributors can format their code, without causing noisy diffs or needing to manually check out unrelated changes
- Contributors with automatic formatting enabled in their development environments will not need to disable this feature to contribute to twiggy
- Coding style will not need to be mentioned in review if it is handled by CI checks
Cons:
- This can potentially make things harder for first time contributors
- Documentation might need to be added to the contribution docs explaining how to set up
rustfmt
IME this can be an initial hurdle for those starting out with a project, but helpful documentation and friendly assistance when these problems occur helps this to outweigh the `cons' of not using automatic formatting.
Note: Labelling this as a question, because I would like to field this for feedback before opening a PR to do this 🙂
Other Note: Along the same lines, how would we feel about using clippy
for lints in the same way? I'm a fan of it, but hesitant to jump directly to both fmt
and clippy
checks, in the event that the linter might be overly strict for the some of the reasons outlined above.
I am in favor 👍
Cool! So, for this to be closed, I think we would need:
- A PR with the changes generated by running
cargo fmt
on the existing code - adjustments to the CI script to check for formatting
- additions to the contribution docs to explain how to install
rustfmt