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

add module-level documentation for vec's in-place iteration #87667

Merged
merged 5 commits into from
Mar 24, 2022
Merged

add module-level documentation for vec's in-place iteration #87667

merged 5 commits into from
Mar 24, 2022

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Jul 31, 2021

As requested in the last libs team meeting and during previous reviews.

Feel free to point out any gaps you encounter, after all non-obvious things may with hindsight seem obvious to me.

r? @yaahc

CC @steffahn

@the8472 the8472 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 31, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2021
@the8472 the8472 force-pushed the document-in-place-iter branch 2 times, most recently from c208d77 to 2b48b28 Compare July 31, 2021 16:55
SkiFire13
SkiFire13 reviewed Jul 31, 2021
View reviewed changes
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Sign in to view
steffahn
steffahn reviewed Aug 4, 2021
View reviewed changes
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Outdated Show resolved Hide resolved
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@the8472
Copy link
Member Author

the8472 commented Aug 26, 2021

The first two commits are now code motion, the 3rd commit contains the documentation changes. It may be easier to review that in isolation.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2021
@bors
Copy link
Contributor

bors commented Oct 5, 2021

☔ The latest upstream changes (presumably #89545) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2021
@yaahc
Copy link
Member

yaahc commented Mar 7, 2022 •
edited
Loading

@the8472 how have you been navigating these docs as you write them? I noticed you have a bunch of cross references to existing specialization traits like TrustedRandomAccessNoCoerce when I started reading through the module doc which aren't part of the PR. I wanted to be able to follow these cross references to help understand the docs you're adding so I tried pulling your pr branch and generating the docs locally but when I tried running RUSTDOCFLAGS="--document-private-items" x doc library/alloc --open it is failing to build because of some unresolved links in unrelated code.

@the8472
Copy link
Member Author

the8472 commented Mar 8, 2022

--document-private-items --document-hidden-items

@yaahc
Copy link
Member

yaahc commented Mar 8, 2022

--document-private-items --document-hidden-items

hmm, didn't seem to fix it but I just manually fixed the one dead link and it runs fine now. Surprised you're not running into the same issue but w/e. I'll check if the same issue is on master later and fix that separately. continues with review

@the8472
Copy link
Member Author

the8472 commented Mar 15, 2022

I also have deny-warnings = false in my config.toml that may affect rustdoc too.

yaahc
yaahc reviewed Mar 15, 2022
View reviewed changes
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/source_iter_marker.rs Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Show resolved Hide resolved
library/alloc/src/vec/in_place_collect.rs Outdated Show resolved Hide resolved
@yaahc
Copy link
Member

yaahc commented Mar 18, 2022

@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot 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 Mar 18, 2022
@the8472
Copy link
Member Author

the8472 commented Mar 23, 2022

@rustbot ready

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

yaahc commented Mar 23, 2022

@the8472, can you mark the comments that have been resolved as such?

@the8472
Copy link
Member Author

the8472 commented Mar 23, 2022

Done, it was all of them.

@yaahc
Copy link
Member

yaahc commented Mar 23, 2022

awesome, thank you very much @the8472!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 23, 2022

📌 Commit 29e29ce has been approved by yaahc

@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 23, 2022
@bors
Copy link
Contributor

bors commented Mar 24, 2022

⌛ Testing commit 29e29ce with merge 6970f88...

@bors
Copy link
Contributor

bors commented Mar 24, 2022

☀️ Test successful - checks-actions
Approved by: yaahc
Pushing 6970f88 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 24, 2022
@bors bors merged commit 6970f88 into rust-lang:master Mar 24, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 24, 2022
@bors bors mentioned this pull request Mar 24, 2022
Closed
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6970f88): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottmcm scottmcm scottmcm left review comments

@SkiFire13 SkiFire13 SkiFire13 left review comments

@steffahn steffahn steffahn left review comments

@yaahc yaahc yaahc left review comments

Assignees

@yaahc yaahc

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

Successfully merging this pull request may close these issues.