rustwasm/twiggy

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

Closed by #253, #254.