Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt These commits modify the If this was intentional then you can ignore this comment. |
Mark-Simulacrum
left a comment
There was a problem hiding this comment.
r=me modulo rustfmt ping
src/tools/rustfmt/Cargo.toml
Outdated
There was a problem hiding this comment.
@rust-lang/rustfmt Are you OK with this landing upstream? It seems like there's some changes to code here as well so maybe we should go through PR to rustfmt?
There was a problem hiding this comment.
Adding clap 3.1 gave us some trouble rust-lang/rustfmt#5395, but I think bumping to 4.2.1 upstream should be fine. I've gone ahead and tested the 4.2.1 changes locally. There are some slight changes in the --help output, but I also think that's fine. @calebcartwright what are your thoughts?
3.1
4.2.1
There was a problem hiding this comment.
In general I don't think subtrees should be updated in tree unless absolutely necessary (e.g. rustc_ast updates requiring corresponding updates in clippy, rustfmt, etc.). I understand the benefit of making a broad update like this in tree, but I don't particularly like it (we run a reduced test suite here, lockfile considerations can complicate subtree syncs, etc.)
Suppose we can let this one slide (thanks Yacin for taking a closer look), but in the future would really want to see something like this come directly to the rustfmt repo and then make its way in-tree here via the usual sync process
|
Thanks for the review. I will send PR to the subtree next time changes like this are to be made. @bors r=Mark-Simulacrum |
|
📌 Commit 915b5803946204a593d7c6cfc42e3431dcb51fa7 has been approved by It is now in the queue for this repository. |
|
☔ The latest upstream changes (presumably #110008) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r=Mark-Simulacrum |
…Mark-Simulacrum bump treewide clap to 4.2.1 cc rust-lang#109302
|
@bors r- failed in rollup |
|
Fixed the error by adding the feature in @bors r=Mark-Simulacrum |
…Mark-Simulacrum bump treewide clap to 4.2.1 cc rust-lang#109302
|
Thanks. I was suspicious this was the reason why it failed but couldn't tell. |
|
@bors rollup=iffy r- Looks like rustc-workspace-hack needs to be updated? |
|
We should wait for #109133 to be merged first such that this is easier. In the meantime perhaps I could send the rustfmt part as a PR to rustfmt first. |
|
☔ The latest upstream changes (presumably #110239) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@fee1-dead #109133 is merged now which unblocks this. |
|
This is still blocked on rust-lang/rustfmt#5749 and a submodule sync. |
|
Closing, as it looks like clap has already been bumped. |


cc #109302