-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix some tidy paper cuts #139867
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
Fix some tidy paper cuts #139867
Conversation
I feel like they are still wrong, but maybe less so .-. The `info:` was unhelpful -- we only use upstream in CI nowdays.
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes seem reasonable to me, and running this locally seems to do what I expect. r=me unless Jakub has change requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one comment, otherwise fine. Some of the git comments will be obsolete after #138591, but that doesn't really matter.
update_rustfmt_version(build); | ||
} | ||
// Update `build/.rustfmt-stamp`, allowing this code to ignore files which has not been changed | ||
// since last merge. This is similar to what `x fmt` does. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit misleading, because this function is actually called from x fmt
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I did not realize that! I just removed the sentence about x fmt
.
If checking succeeded, it's equivalent to successfully formatting.
@bors r=jieyouxu |
Fix some tidy paper cuts The main thing this fixes is that currently, if you run `x t tidy` it will format ~6K files, even though it's supposed to format only modified files (whatever this is a useful optimization or not is besides the point). The problem is that `x t tidy` never writes the `rustfmt` stamp, so it always assumes `rustfmt` that was last used is outdated and we need to recheck everything. This PR fixes it by actually writing the stamp. There are also some minor tweaks to comments/diagnostics. cc `@Kobzol` this probably conflicts with rust-lang#138591. I didn't fix anything, just tried to document better the status quo. r? `@jieyouxu`
Rollup of 8 pull requests Successful merges: - rust-lang#136926 (Stabilize `-Zdwarf-version` as `-Cdwarf-version`) - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options) - rust-lang#139853 (Disable combining LLD with external llvm-config) - rust-lang#139867 (Fix some tidy paper cuts) - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block) - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT) - rust-lang#139884 (Update books) - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks) r? `@ghost` `@rustbot` modify labels: rollup
Fix some tidy paper cuts The main thing this fixes is that currently, if you run `x t tidy` it will format ~6K files, even though it's supposed to format only modified files (whatever this is a useful optimization or not is besides the point). The problem is that `x t tidy` never writes the `rustfmt` stamp, so it always assumes `rustfmt` that was last used is outdated and we need to recheck everything. This PR fixes it by actually writing the stamp. There are also some minor tweaks to comments/diagnostics. cc ``@Kobzol`` this probably conflicts with rust-lang#138591. I didn't fix anything, just tried to document better the status quo. r? ``@jieyouxu``
Rollup of 10 pull requests Successful merges: - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options) - rust-lang#139823 (Fix some bootstrap papercuts) - rust-lang#139853 (Disable combining LLD with external llvm-config) - rust-lang#139867 (Fix some tidy paper cuts) - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block) - rust-lang#139876 (Make CodeStats' type_sizes public) - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT) - rust-lang#139884 (Update books) - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks) - rust-lang#139893 (Add test for issue 125668) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options) - rust-lang#139823 (Fix some bootstrap papercuts) - rust-lang#139867 (Fix some tidy paper cuts) - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block) - rust-lang#139876 (Make CodeStats' type_sizes public) - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT) - rust-lang#139884 (Update books) - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks) - rust-lang#139893 (Add test for issue 125668) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options) - rust-lang#139823 (Fix some bootstrap papercuts) - rust-lang#139867 (Fix some tidy paper cuts) - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block) - rust-lang#139876 (Make CodeStats' type_sizes public) - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT) - rust-lang#139884 (Update books) - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks) - rust-lang#139893 (Add test for issue 125668) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#139867 - WaffleLapkin:tidypaper, r=jieyouxu Fix some tidy paper cuts The main thing this fixes is that currently, if you run `x t tidy` it will format ~6K files, even though it's supposed to format only modified files (whatever this is a useful optimization or not is besides the point). The problem is that `x t tidy` never writes the `rustfmt` stamp, so it always assumes `rustfmt` that was last used is outdated and we need to recheck everything. This PR fixes it by actually writing the stamp. There are also some minor tweaks to comments/diagnostics. cc ```@Kobzol``` this probably conflicts with rust-lang#138591. I didn't fix anything, just tried to document better the status quo. r? ```@jieyouxu```
|
@Kobzol I don't think this changed but ur welcome :) |
Hmm, maybe it was confirmation bias =D But tidy felt faster too :) |
The main thing this fixes is that currently, if you run
x t tidy
it will format ~6K files, even though it's supposed to format only modified files (whatever this is a useful optimization or not is besides the point). The problem is thatx t tidy
never writes therustfmt
stamp, so it always assumesrustfmt
that was last used is outdated and we need to recheck everything. This PR fixes it by actually writing the stamp.There are also some minor tweaks to comments/diagnostics. cc @Kobzol this probably conflicts with #138591. I didn't fix anything, just tried to document better the status quo.
r? @jieyouxu