-
Notifications
You must be signed in to change notification settings - Fork 13.4k
bootstrap: Clean up try_run #113643
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
bootstrap: Clean up try_run #113643
Conversation
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 PR alone reduces a lot of confusion. Thanks a lot! :)
Only 1 suggestion which is non-blocker. Feel free to queue this up with r=me if you don't agree on the suggestion.
@bors r=ozkanonur |
📌 Commit e99d99a7eec7431d4c1984cd93a0ad32283fa2ab has been approved by It is now in the queue for this repository. |
⌛ Testing commit e99d99a7eec7431d4c1984cd93a0ad32283fa2ab with merge f295a360b0630b5c1d5735d6904e24390dafd3db... |
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #113514) made this pull request unmergeable. Please resolve the merge conflicts. |
This does three things: 1. Remove `forward!(Build, fn try_run())`. Having `try_run` behave differently as a free function than an associated function is confusing, and `Builder::try_run` is a very desirable name. 2. Move `test::try_run` and `run::try_run` to `Builder::try_run`. These functions are different than `Config::try_run` - they delay the failure and print it out at the end of the build. 3. Mark `Config::try_run` as deprecated to encourage people to use `Builder::try_run` instead.
It was only used when a `builder` is available, and I want to encourage using the version that supports `--no-fail-fast`.
@bors r=ozkanonur |
Rollup of 3 pull requests Successful merges: - rust-lang#113643 (bootstrap: Clean up try_run) - rust-lang#113731 (Remove unused `bootstrap::util::CiEnv` enum) - rust-lang#113737 (update mailmap for myself) r? `@ghost` `@rustbot` modify labels: rollup
r? @ozkanonur since you reviewed @GuillaumeGomez's PR
i recommend reviewing commit-by-commit