-
Notifications
You must be signed in to change notification settings - Fork 13.4k
tests/ui
: A New Order [3/N]
#141965
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
tests/ui
: A New Order [3/N]
#141965
Conversation
(If you find this too much friction, squashing and thus using just one single commit from the very beginning is also fine with me. You do not have to do the following at all!) To make review easier for these kinds of test changes, I usually try to keep the commits atomic by reblessing the one modified test (since we're not modifying compiler/std), so that the reviewer can immediately tell that no unexpected stderr is changed, instead of only doing reblesses in the end. (Of course, this takes more rebase effort and is more annoying on merge conflicts.) (Don't bother doing that for this PR ofc.) |
@jieyouxu honestly, reblessing each test (if needed) when it's changed is acutally easier than doing it all in a final commit, im not sure why i initially thought doing it at the end would make the review simpler, but it's good to know that hadling it per-commit is not only better for review but also easier to manage overall |
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.
Thanks, minor nits, and please squash.
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
@rustbot ready |
Sorry, one more nit because I mistyped the path (#141965 (comment)). @rustbot author |
@rustbot ready |
@bors r+ rollup |
`tests/ui`: A New Order [3/N] Some `tests/ui/` housekeeping, to trim down number of tests directly under `tests/ui/`. Part of rust-lang#133895. r? `@jieyouxu`
Rollup of 8 pull requests Successful merges: - #140638 (UnsafePinned: also include the effects of UnsafeCell) - #141272 (modularize the config module bootstrap) - #141777 (Do not run PGO/BOLT in x64 Linux alt builds) - #141870 (Fix broken link to rustc_type_ir module in serialization docs) - #141938 (update rust offload bootstrap) - #141962 (rustc-dev-guide subtree update) - #141965 (`tests/ui`: A New Order [3/N]) - #141970 (implement new `x` flag: `--skip-std-check-if-no-download-rustc`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 6 pull requests Successful merges: - #140638 (UnsafePinned: also include the effects of UnsafeCell) - #141777 (Do not run PGO/BOLT in x64 Linux alt builds) - #141938 (update rust offload bootstrap) - #141962 (rustc-dev-guide subtree update) - #141965 (`tests/ui`: A New Order [3/N]) - #141970 (implement new `x` flag: `--skip-std-check-if-no-download-rustc`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 6 pull requests Successful merges: - rust-lang/rust#140638 (UnsafePinned: also include the effects of UnsafeCell) - rust-lang/rust#141777 (Do not run PGO/BOLT in x64 Linux alt builds) - rust-lang/rust#141938 (update rust offload bootstrap) - rust-lang/rust#141962 (rustc-dev-guide subtree update) - rust-lang/rust#141965 (`tests/ui`: A New Order [3/N]) - rust-lang/rust#141970 (implement new `x` flag: `--skip-std-check-if-no-download-rustc`) r? `@ghost` `@rustbot` modify labels: rollup
Some
tests/ui/
housekeeping, to trim down number of tests directly undertests/ui/
. Part of #133895.r? @jieyouxu