-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Remove Reflect #39075
Remove Reflect #39075
Conversation
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
I'm not 100% sure of the comment change in |
☔ The latest upstream changes (presumably #39071) made this pull request unmergeable. Please resolve the merge conflicts. |
Rebased. |
☔ The latest upstream changes (presumably #39243) made this pull request unmergeable. Please resolve the merge conflicts. |
@nikomatsakis you said that you've started a crater run on this PR. Are there any results in yet? Also, will me rebasing this PR in any way inconvenience the crater run? I don't want to force-push and destroy something :) |
@est31 let me check :) also, rebase away |
@est31 something went wrong with my run :( |
* Remove the Reflect trait * Remove the "reflect" lang feature
@nikomatsakis rebased. |
I will crater it. |
the three root regressions reported by crater all are pretty obvious (each crate is importing |
@bors r+ |
📌 Commit af46d69 has been approved by |
The feature is deprecated and removal is the plan, and the PR looks good; impact is minimal. So I gave r+! |
Remove Reflect PR for removing the `Reflect` trait. Opened so that a crater run can be done for testing the impact: #27749 (comment) Fixes #27749
☀️ Test successful - status-appveyor, status-travis |
PR for removing the
Reflect
trait. Opened so that a crater run can be done for testing the impact: #27749 (comment)Fixes #27749