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

Add cargo fmt to CI and delete rustfmt.toml #5176

Merged
merged 4 commits into from
Mar 15, 2018

Conversation

alexcrichton
Copy link
Member

This commit adds CI to run cargo fmt over Cargo itself as well as the internal
crates-io crate. This should switch Cargo to the "default style" (aka whatever
rustfmt spits out) and ensure that we keep it that way via CI. Hopefully this
won't be too much of a bother to keep up and running in CI as it should just be
a cargo fmt away!

@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

Spot checking this it all looks pretty good to me, but it definitely wreaks havoc on the formatting in the tests. I figure we can try to touch things up there as we go along?

@matklad
Copy link
Member

matklad commented Mar 14, 2018

I've managed to fix horrendous formatting of single line strings with a bit of regex magic and reformatting with rustfmt again, you might want to cherry-pick this commit: matklad@eb85b79

I have no idea what to do with multiline strings, I am not sure that "we can try to touch things up there as we go along", because every touch-up I've tried didn't produce a satisfactory result. But multiline strings don't look that bad. Also, not that our tests were particularly pretty beforehand :)

Could we remove that sh script that checked for line lengths? Or is still needed for docs?

@alexcrichton
Copy link
Member Author

Nice! I'll definitely throw that on before merging.

I also agree that the tests haven't really ever been too too pretty. I'm not too too worried in that tests are quite commonly write-once-read-never, but I'm also totally ok jumping on the "blindly run on rustfmt and don't worry" bandwagon.

I had forgotten we had a line-length script, although it looks like it's not actually run anywhere! I'll delete it here

@matklad
Copy link
Member

matklad commented Mar 14, 2018

r=me with script removed and the patch applied!

Overall, this looks awesome, but that's because I am a

frobnicate(
    spam,
    eggs,
)

person myself, rather than

frobnicate(spam,
           eggs)

:)

@bors
Copy link
Contributor

bors commented Mar 14, 2018

☔ The latest upstream changes (presumably #5181) made this pull request unmergeable. Please resolve the merge conflicts.

alexcrichton and others added 3 commits March 14, 2018 17:47
This commit adds CI to run `cargo fmt` over Cargo itself as well as the internal
`crates-io` crate. This should switch Cargo to the "default style" (aka whatever
rustfmt spits out) and ensure that we keep it that way via CI. Hopefully this
won't be too much of a bother to keep up and running in CI as it should just be
a `cargo fmt` away!
@alexcrichton
Copy link
Member Author

@bors: r+

Say goodbye to everything in Cargo's queue...

@bors
Copy link
Contributor

bors commented Mar 15, 2018

📌 Commit b0c181d has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 15, 2018

⌛ Testing commit b0c181d with merge 02918fafd5a469afada0094e0fae1f017195177e...

@bors
Copy link
Contributor

bors commented Mar 15, 2018

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 15, 2018

📌 Commit 358b9f7 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 15, 2018

⌛ Testing commit 358b9f79db97025d476ce3260714d2a9f74517f2 with merge a12dd42da5b8e6ced0ac345ed319a64d95041032...

@bors
Copy link
Contributor

bors commented Mar 15, 2018

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member Author

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 15, 2018

📌 Commit 3d85814 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 15, 2018

⌛ Testing commit 3d85814 with merge 805fbeb...

bors added a commit that referenced this pull request Mar 15, 2018
Add `cargo fmt` to CI and delete `rustfmt.toml`

This commit adds CI to run `cargo fmt` over Cargo itself as well as the internal
`crates-io` crate. This should switch Cargo to the "default style" (aka whatever
rustfmt spits out) and ensure that we keep it that way via CI. Hopefully this
won't be too much of a bother to keep up and running in CI as it should just be
a `cargo fmt` away!
@bors
Copy link
Contributor

bors commented Mar 15, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 805fbeb to master...

@alexcrichton alexcrichton deleted the rustfmt branch April 25, 2018 19:28
bors added a commit that referenced this pull request Sep 27, 2020
Normalize raw string indentation.

It has always slightly bugged me how strings were indented after rustfmt was run across the repo (#5176). This attempts to normalize the strings so that they open and close on the same column.  This only touches the `tests` directory.
@ehuss ehuss added this to the 1.26.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants