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

Fix documentation of conversion from String to OsString #83700

Merged

Conversation

steffahn
Copy link
Member

From this question on URLO, I noticed that the documentation of From<String> for OsString incorrectly claims to be copying data.

@rustbot modify labels: T-doc, T-libs

@rustbot rustbot added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 31, 2021
@steffahn
Copy link
Member Author

I’m also adding a few more minor cleanup things to this PR regarding From implementations

@steffahn steffahn force-pushed the string_to_pathbuf_conversion_documentation branch from 6dc7d1f to 5264ef2 Compare March 31, 2021 11:34
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Good catch!

I’m also adding a few more minor cleanup things to this PR regarding From implementations

Do you have more cleanups other than 5264ef2 on this PR?

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 31, 2021
@steffahn
Copy link
Member Author

Do you have more cleanups other than 5264ef2 on this PR?

I can try to do some more “cleanups” like that, haven’t put in the work so far... most notably there’s also more problems with some of the documentations about string conversion, e.g. OsString to Rc<OsStr> (and a few more) claim to be working “without copying or allocating”.

@steffahn
Copy link
Member Author

steffahn commented Mar 31, 2021

Ah, in case you were just asking about what I was referring to, no I haven’t got anything beyond 5264ef2 yet. Anything extra can also go into a new PR if this one gets approved in the meantime.

@steffahn
Copy link
Member Author

Wait, @rust-highfive didn’t even assign anyone!?
@rustbot ping infra

@rustbot
Copy link
Collaborator

rustbot commented Mar 31, 2021

Error: Only Rust team members can ping teams.

Please let @rust-lang/release know if you're having trouble with this bot.

@steffahn
Copy link
Member Author

well, okay, that didn’t work, @pietroalbini

Highfive failed to assign someone here. I'll add more logging so we'll discover what's causing this in the future.

When something like this happens again, please ping the whole infra team. Thanks :)

#75911 (comment)

@steffahn
Copy link
Member Author

Only Rust team members can ping teams.

Guess I can do it manually
@aidanhs @Mark-Simulacrum @kennytm @pietroalbini @shepmaster

@Dylan-DPC-zz
Copy link

please do not ping people :D yeah it's a known thing and sometimes it happens. The infra team is aware about it.

r? @Dylan-DPC

@shepmaster
Copy link
Member

s/documenation/documentation/g

@jyn514 jyn514 changed the title Fix documenation of conversion from String to OsString Fix documentation of conversion from String to OsString Mar 31, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 31, 2021

s/documenation/documentation/g

One step ahead of you ;)

@JohnTitor
Copy link
Member

Ah, in case you were just asking about what I was referring to, no I haven’t got anything beyond 5264ef2 yet. Anything extra can also go into a new PR if this one gets approved in the meantime.

Yeah, that's what I meant. Okay, then @bors r+ rollup
Thanks!

@bors
Copy link
Contributor

bors commented Mar 31, 2021

📌 Commit 5264ef2a8063ab539f11e0081851a00b5c1ef62d has been approved by JohnTitor

@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 31, 2021
@shepmaster
Copy link
Member

One step ahead of you ;)

It's also in the commit message, which is why I commented instead of changing it myself.

@steffahn steffahn force-pushed the string_to_pathbuf_conversion_documentation branch from 5264ef2 to f5e7dbb Compare March 31, 2021 14:03
@steffahn
Copy link
Member Author

removed typo in the commit message

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

I was late a bit but...

library/std/src/path.rs Outdated Show resolved Hide resolved
library/std/src/path.rs Outdated Show resolved Hide resolved
More links, one more occurrence of “a OsString”

Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
@steffahn steffahn force-pushed the string_to_pathbuf_conversion_documentation branch from eb3228e to 7509aa1 Compare March 31, 2021 14:09
@JohnTitor
Copy link
Member

JohnTitor commented Mar 31, 2021

Thanks again :)
@bors r+

@bors
Copy link
Contributor

bors commented Mar 31, 2021

📌 Commit 7509aa1 has been approved by JohnTitor

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 31, 2021
…on_documentation, r=JohnTitor

Fix documentation of conversion from String to OsString

From [this question on URLO](https://users.rust-lang.org/t/does-converting-a-string-into-a-pathbuf-allocate-new-buffer/57678), I noticed that the documentation of `From<String> for OsString` incorrectly claims to be copying data.

`@rustbot` modify labels: T-doc, T-libs
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 31, 2021
…on_documentation, r=JohnTitor

Fix documentation of conversion from String to OsString

From [this question on URLO](https://users.rust-lang.org/t/does-converting-a-string-into-a-pathbuf-allocate-new-buffer/57678), I noticed that the documentation of `From<String> for OsString` incorrectly claims to be copying data.

``@rustbot`` modify labels: T-doc, T-libs
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 31, 2021
…on_documentation, r=JohnTitor

Fix documentation of conversion from String to OsString

From [this question on URLO](https://users.rust-lang.org/t/does-converting-a-string-into-a-pathbuf-allocate-new-buffer/57678), I noticed that the documentation of `From<String> for OsString` incorrectly claims to be copying data.

```@rustbot``` modify labels: T-doc, T-libs
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 1, 2021
Rollup of 5 pull requests

Successful merges:

 - rust-lang#83015 (Add regression tests for rust-lang#79825 and rust-lang#81555)
 - rust-lang#83699 (Add a regression test for issue-68830)
 - rust-lang#83700 (Fix documentation of conversion from String to OsString)
 - rust-lang#83711 (Clarify `--print target-list` is a rustc's option)
 - rust-lang#83712 (Update LLVM with another wasm simd fix)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ea277f1 into rust-lang:master Apr 1, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 1, 2021
@steffahn steffahn deleted the string_to_pathbuf_conversion_documentation branch April 1, 2021 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants