-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Move RangeArgument and Bound to libcore. #41460
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
Thanks for the PR! We’ll periodically check in on it to make sure that @aturon or someone else from the team reviews it soon. It also looks like there are some compilation failures, so perhaps you could take a look at those? |
4c65c78
to
f12fa92
Compare
f12fa92
to
688cbda
Compare
I think that everything should be fixed now! |
Looks like that's not quite true: some feature attributes need to be added to a few crates: https://travis-ci.org/rust-lang/rust/jobs/225394483#L780. This is somewhat concerning, as that implies that moving this feature changed from stable to unstable? Or otherwise made things "now unstable," which feels like a regression? Might want to pay careful attention to the stability attributes... not certain. |
cc @rust-lang/libs, I believe this change is in line with our general philosophy around |
This seems reasonable to me, but note that it's adding a new insta-stable type, What's the rationale for the movement to justify the duplication? |
I hadn't considered this, and you're right. I could make a |
@aturon it's been awhile since you last looked at this, mind giving it another look with an eye towards reviewing? |
@clarcharr Looks like there are quite a few compilation failures due to feature gating, could you take a look? |
@clarcharr Thanks for the contribution! The change looks good to me but it needs some changes still, see the Travis failure. Do you think you'll have a chance to make them soon? |
Hi @clarcharr, thank you for your PR! Since there hasn't been any activity on this recently, we're going to close this to keep our open PR list current, but please feel free to reopen this when you have time to work on this again ❤️ |
This moves them to
core::ops
with the rest of theRange
types and re-exports them incollections
.Right now, there's no reason why these shouldn't be accessible in libcore. Caution is due, however, because this would add
Bound
to libcore as stable immediately.Also as a side note, it may be desirable in the future to split up
libcore/ops.rs
as it is getting rather big.