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

std: Add capacity guarantees notes for OsString #95394

Closed
wants to merge 1 commit into from

Conversation

Xuanwo
Copy link
Contributor

@Xuanwo Xuanwo commented Mar 28, 2022

This PR intends to stabilize feature try_reserve_2, part of #91789

This PR will is split from #95392

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(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 28, 2022
@Xuanwo
Copy link
Contributor Author

Xuanwo commented Mar 28, 2022

@rustbot modify labels: +T-libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 28, 2022
@Xuanwo
Copy link
Contributor Author

Xuanwo commented Mar 28, 2022

r? @dtolnay

@dtolnay dtolnay added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Mar 28, 2022
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.

I think copying the same cursory 1-sentence comment into 7 different places is not the best way to accomplish this documentation. I would prefer to see a single section that explains this topic more clearly in detail, probably with example code to illustrate some correct and incorrect uses of the APIs where this guarantee is relevant. Especially showing the incorrect case is important to illustrate the limitations of this guarantee (the "not specified" case).

This section would probably be most appropriate on the OsString type but alternatively on the capacity() method could work too, depending on how you write it. Everywhere else where that content is important to understand can just link to it.

@dtolnay dtolnay added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2022
@joshtriplett
Copy link
Member

Updated version in #97202 .

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 15, 2022
…ntation, r=dtolnay

os str capacity documentation

This is based on rust-lang#95394 , with expansion and consolidation
to address comments from `@dtolnay` and other `@rust-lang/libs-api` team members.
workingjubilee pushed a commit to tcdi/postgrestd that referenced this pull request Sep 15, 2022
…r=dtolnay

os str capacity documentation

This is based on rust-lang/rust#95394 , with expansion and consolidation
to address comments from `@dtolnay` and other `@rust-lang/libs-api` team members.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants