-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
#53576 Renaming TyAnon -> TyOpaque #53705
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
r? @eddyb |
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 looks great to me! (@eddyb might remember if there's anywhere else that needs changing, so I'll leave the final approval to them.)
All good? Should I be doing anything else? |
☔ The latest upstream changes (presumably #53616) made this pull request unmergeable. Please resolve the merge conflicts. |
How do I resolve these conflicts without merging from upstream? Edit: Just rebased and it should work |
Ping from triage @eddyb / @rust-lang/compiler: This PR requires your review. |
Sorry, I meant to get back to this, but somehow forgot! It all looks correct to me — just one last thing: could you squash the related changes together? (Either one commit for everything, or one for code changes and one for test changes.) I'll approve after that (and @eddyb can add any additional comments afterwards if I've forgotten anything else)! Thanks! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I squashed into two commits, thanks for the update! |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Does anyone know what this error is referencing? Edit: I thought I had ran all of the normal test commands and they passed (?) |
@bors retry network error |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
You updated the llvm submodule. Please revert that (That's also the source of the failure) |
Yeah for some reason I updated a lot of the submodules somewhere in the rebasing process and reverted all but that one. My bad it should work now |
@bors r+ |
📌 Commit f4d4faa has been approved by |
rust-lang#53576 Renaming TyAnon -> TyOpaque Fixes rust-lang#53576
☀️ Test successful - status-appveyor, status-travis |
Fixes #53576