Skip to content
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

Expose str::SplitInclusive in alloc and therefore in std #83408

Merged
merged 2 commits into from
Mar 24, 2021

Conversation

ijackson
Copy link
Contributor

@ijackson ijackson commented Mar 23, 2021

This seems to have been omitted from the beginning when this feature was first introduced in 86bf962. Most users won't need to name this type which is probably why this wasn't noticed in the meantime.

See #83372 for a different but related bug.

Notes for reviewers

I think I have got this right but TBH I am not very familiar with the relationship between core and std and so on. I also haven't don't any kind of test (not even a build) yet. I will do a local docs build to see that the type now appears in the std docs. I did a local docs build and it has made this type appear as std::str::SplitInclusive as expected

The linkification of the return value from str::split_inclusive teleports me to the online url for core::str::SplitInclusive. I think this may be a rustdoc anomaly (similar to #79630 maybe) but I am not sure. Perhaps it means I haven't done the std -> core referrence correctly.

I made this insta-stable since it seems like simply a bug. Please LMK if that is not right. (edited to add:) In particular, IDK how this ought to relate to the (?)current release process.

This seems to have been omitted from the beginning when this feature
was first introduced in 86bf962.

Most users won't need to name this type which is probably why this
wasn't noticed in the meantime.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2021
@ijackson
Copy link
Contributor Author

Hrm, I should have said

r? @Mark-Simulacrum

I think. I wonder if I can do that now.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I agree this was simply an oversight.

@dtolnay
Copy link
Member

dtolnay commented Mar 24, 2021

Bumping stability attribute to the release corresponding to current nightly (1.53.0) since I don't see this warranting a backport, although we certainly could if someone needs it.

@dtolnay dtolnay assigned dtolnay and unassigned Mark-Simulacrum Mar 24, 2021
@dtolnay
Copy link
Member

dtolnay commented Mar 24, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 24, 2021

📌 Commit 633a66f has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2021
@bors
Copy link
Contributor

bors commented Mar 24, 2021

⌛ Testing commit 633a66f with merge 680d9fc...

@bors
Copy link
Contributor

bors commented Mar 24, 2021

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 680d9fc to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 24, 2021
@bors bors merged commit 680d9fc into rust-lang:master Mar 24, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 24, 2021
@ijackson ijackson deleted the expose-splitinclusive branch August 24, 2021 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants