Skip to content

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Jun 13, 2019

No description provided.

@rust-highfive
Copy link

r? @Eh2406

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2019
@Eh2406
Copy link
Contributor

Eh2406 commented Jun 13, 2019

We have tried this before and did not like it, but fmt is much more stable now. @alexcrichton thoughts?

@ehuss
Copy link
Contributor

ehuss commented Jun 13, 2019

I'm generally in favor of this. I think rustfmt is stable enough it probably shouldn't be a problem.

The main issue is the churn it causes on PRs, but I personally don't feel it is too bad. It would be nice if we had a bot that would leave a message when CI fails, particularly for this case, with nice instructions on what to do. We could also use a pre-existing bot like https://www.travisbuddy.com/. I suspect rust-log-analyzer isn't geared towards working on other repos, but maybe that could be used. I don't think that should gate the decision.

For more context, here is the draft RFC for doing the same for rust-lang/rust which mentions adding bot support for pushing rustfmt changes: Centril/rfcs#21. I would think that is more trouble than it's worth. Issuing a command, waiting for it to finish, pulling the changes — sounds more complex than just running cargo fmt locally and then pushing.

@alexcrichton
Copy link
Member

I'm willing to try this out now that rustfmt is more stable. I don't think it's a "perfect solution" but anything going more in depth in this is a huge amount more effort. I'd prefer though that the formatting check was a separate job in Travis rather than tacked onto an existing one

@tesuji
Copy link
Contributor Author

tesuji commented Jun 13, 2019

Done.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 13, 2019

📌 Commit b01f595 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2019
@bors
Copy link
Contributor

bors commented Jun 13, 2019

⌛ Testing commit b01f595 with merge fa05862...

bors added a commit that referenced this pull request Jun 13, 2019
ci: Run cargo fmt on all workspaces
@bors
Copy link
Contributor

bors commented Jun 13, 2019

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

@bors bors merged commit b01f595 into rust-lang:master Jun 13, 2019
@tesuji tesuji deleted the fmt-workspace branch June 14, 2019 01:09
bors added a commit to rust-lang/rust that referenced this pull request Jun 24, 2019
Update cargo

17 commits in 807429e1b6da4e2ec52488ef2f59e77068c31e1f..4c1fa54d10f58d69ac9ff55be68e1b1c25ecb816
2019-06-11 14:06:10 +0000 to 2019-06-24 11:24:18 +0000
- Fix typo in comment (rust-lang/cargo#7066)
- travis: enforce formatting of subcrates as well (rust-lang/cargo#7063)
- _cargo: Make function style consistent (rust-lang/cargo#7060)
- Update some fix comments. (rust-lang/cargo#7061)
- Stabilize default-run (rust-lang/cargo#7056)
- Fix typo in comment (rust-lang/cargo#7054)
- fix(fingerpring): do not touch intermediate artifacts (rust-lang/cargo#7050)
- Resolver test/debug cleanup (rust-lang/cargo#7045)
- Rename to_url → into_url (rust-lang/cargo#7048)
- Remove needless lifetimes (rust-lang/cargo#7047)
- Revert test directory cleaning change. (rust-lang/cargo#7042)
- cargo book /reference/manifest: fix typo (rust-lang/cargo#7041)
- Extract resolver tests to their own crate (rust-lang/cargo#7011)
- ci: Do not install addons on rustfmt build jobs (rust-lang/cargo#7038)
- Support absolute paths in dep-info files (rust-lang/cargo#7030)
- ci: Run cargo fmt on all workspaces (rust-lang/cargo#7033)
- Deprecated ONCE_INIT in favor of Once::new() (rust-lang/cargo#7031)
@ehuss ehuss added this to the 1.37.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants