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.

Sign up for GitHub

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

Jump to bottom

[idea] Use rustfmt in CI for style checks #770

Closed
marmistrz opened this issue Sep 18, 2017 · 6 comments · Fixed by #1854
Closed

[idea] Use rustfmt in CI for style checks #770

marmistrz opened this issue Sep 18, 2017 · 6 comments · Fixed by #1854
Labels
A-testing

Comments

@marmistrz
Copy link
Contributor

This could prevent many style review comments in PRs.
@Susurrus
Copy link
Contributor

I used to run this for my projects but found that it was unstable. I don't believe it's reasonable at this point to use rustfmt in CI, especially since it'll likely point out many errors unrelated to the current PR. I think this would be a good thing to do for 1.0, however. Same with running Clippy on all PRs.

@dingxiangfei2009
Copy link
Contributor

@Susurrus Shall we reconsider enabling this check today? I mean rustfmt is pretty stable now in 2019.

@posborne
Copy link
Member

I think enabling a rustfmt check in CI at this point would be very reasonable and is something I have started to add to a lot of projects including ones with significant churn at the day job.

This can be done via cargo fmt --all -- --check.

@costinsin costinsin mentioned this issue Jun 19, 2022
Merged
bors bot added a commit that referenced this issue Jun 24, 2022
@bors @costinsin
Merge #1748
a45e9f8 
1748: Add format test to CI r=rtzoeller a=costinsin

To enforce uniformity for all PRs, the CI checks if the code
is formatted right using `cargo fmt` tool.

Results after implementing the format test in CicleCI, but before fixing the format errors: https://cirrus-ci.com/build/4684991404703744
Results after fixing the format errors: https://cirrus-ci.com/build/5423803479097344

Solves #770 

Co-authored-by: Costin-Robert Sin <[email protected]>
@SUPERCILEX
Copy link
Contributor

@rtzoeller Pretty sure this was fixed by #1748

@pacak
Copy link
Contributor

pacak commented Nov 6, 2022

I'm not sure. Try checking out and running cargo fmt -- src/*.rs. I'm getting a big diff if I do this.

@SUPERCILEX
Copy link
Contributor

Huh, it seems like we're running into this issue: rust-lang/rustfmt#3253

@SUPERCILEX SUPERCILEX mentioned this issue Nov 6, 2022
Merged
@bors bors bot closed this as completed in 126112c Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reformat everything SUPERCILEX/nix
6 participants
@posborne @pacak @Susurrus @marmistrz @dingxiangfei2009 @SUPERCILEX