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.

Sign up for GitHub

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

Jump to bottom

Improve Rc::new_cyclic and Arc::new_cyclic documentation #95843

Merged
merged 1 commit into from
May 5, 2022
Merged

Improve Rc::new_cyclic and Arc::new_cyclic documentation #95843

merged 1 commit into from
May 5, 2022

Conversation

GuillaumeGomez
Copy link
Member

Fixes #95672.

cc @CAD97 (since I used your explanations)

@GuillaumeGomez GuillaumeGomez added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Apr 9, 2022
@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 Apr 9, 2022
@GuillaumeGomez GuillaumeGomez mentioned this pull request Apr 9, 2022
Closed
wwylele
wwylele reviewed Apr 9, 2022
View reviewed changes
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Sign in to view
mejrs
mejrs reviewed Apr 9, 2022
View reviewed changes
Copy link
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

Thanks, this api can really use some better explanations ❤️

library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
mejrs
mejrs reviewed Apr 10, 2022
View reviewed changes
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@dtolnay
Copy link
Member

dtolnay commented Apr 16, 2022

r? rust-lang/libs

@rust-highfive rust-highfive assigned m-ou-se and unassigned dtolnay Apr 16, 2022
@GuillaumeGomez
Copy link
Member Author

Ping @m-ou-se

@m-ou-se
Copy link
Member

m-ou-se commented May 3, 2022

Apologies for the delay. Reviewing now.

m-ou-se
m-ou-se reviewed May 3, 2022
View reviewed changes
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 3, 2022
@GuillaumeGomez
Copy link
Member Author

Applied @m-ou-se suggestions and removed the unneeded parts.

m-ou-se
m-ou-se reviewed May 3, 2022
View reviewed changes
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

Updated!

m-ou-se
m-ou-se reviewed May 4, 2022
View reviewed changes
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

Updated!

@m-ou-se
Copy link
Member

m-ou-se commented May 5, 2022

@bors r+ rollup

Thanks for putting up with all my comments. ^^'

@bors
Copy link
Contributor

bors commented May 5, 2022

📌 Commit 67d1e7b has been approved by m-ou-se

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 5, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 5, 2022
@GuillaumeGomez
Copy link
Member Author

No problem, they're very welcome! Thanks for putting up with this doc improvement. ;)

@matthiaskrgr matthiaskrgr mentioned this pull request May 5, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 5, 2022
@bors
Auto merge of rust-lang#96734 - matthiaskrgr:rollup-hng33tb, r=matthi…
50cf76c 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#95359 (Update `int_roundings` methods from feedback)
 - rust-lang#95843 (Improve Rc::new_cyclic and Arc::new_cyclic documentation)
 - rust-lang#96507 (Suggest calling `Self::associated_function()`)
 - rust-lang#96635 (Use "strict" mode in JS scripts)
 - rust-lang#96673 (Report that opaque types are not allowed in impls even in the presence of other errors)
 - rust-lang#96682 (Show invisible delimeters (within comments) when pretty printing.)
 - rust-lang#96714 (interpret/validity: debug-check ScalarPair layout information)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f07303 into rust-lang:master May 5, 2022
@rustbot rustbot added this to the 1.62.0 milestone May 5, 2022
@GuillaumeGomez GuillaumeGomez deleted the improve-new-cyclic-doc branch May 5, 2022 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mejrs mejrs mejrs left review comments

@wwylele wwylele wwylele left review comments

@m-ou-se m-ou-se m-ou-se left review comments

Assignees

@m-ou-se m-ou-se

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 Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.

{Rc,Arc}::new_cyclic documentation is insufficient
9 participants
@GuillaumeGomez @rust-highfive @rust-log-analyzer @dtolnay @m-ou-se @bors @wwylele @mejrs @rustbot