Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task: rustfmt #1429

Open
dhardy opened this issue Apr 3, 2024 · 5 comments
Open

Task: rustfmt #1429

dhardy opened this issue Apr 3, 2024 · 5 comments
Labels
E-easy Participation: easy job

Comments

@dhardy
Copy link
Member

dhardy commented Apr 3, 2024

We should apply rustfmt to the project, probably with #[rustfmt::skip] in some places, and add a CI check. It's going to conflict with a bunch of open PRs, but we always have some.

@MichaelOwenDyer are you interested?

@dhardy dhardy added the E-easy Participation: easy job label Apr 3, 2024
@MichaelOwenDyer
Copy link
Member

Sure thing. How do you feel about a pre-commit hook as well?

@dhardy
Copy link
Member Author

dhardy commented Apr 3, 2024

Thanks. While format-before-commit is good practice, I'm not keen on the idea of tools changing code right before a commit, so no.

@dhardy
Copy link
Member Author

dhardy commented Apr 3, 2024

Sorry, it's probably worth getting #1424 sorted before a more general reformat.

@newpavlov
Copy link
Member

newpavlov commented Apr 3, 2024

It's also worth to add Clippy job to CI (with a fixed toolchain version) and fix existing warnings (29 on current Nightly) in the same PR.

@MichaelOwenDyer
Copy link
Member

MichaelOwenDyer commented Apr 4, 2024

I ran rustfmt locally to see how severe the changes would be and it looks generally quite manageable, I only identified a couple files where we might want to sprinkle in #[rustfmt::skip]. I'll wait for #1424 to be merged before opening a PR.

It's also worth to add Clippy job to CI (with a fixed toolchain version) and fix existing warnings (29 on current Nightly) in the same PR.

Good idea 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Participation: easy job
Projects
None yet
Development

No branches or pull requests

3 participants