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

Split each iterator adapter and source into individual modules #77697

Merged
merged 6 commits into from
Nov 23, 2020
Merged

Split each iterator adapter and source into individual modules #77697

merged 6 commits into from
Nov 23, 2020

Conversation

WaffleLapkin
Copy link
Member

This PR creates individual modules for each iterator adapter and iterator source.

This is done to enhance the readability of corresponding modules (adapters/mod.rs and sources.rs) which were hard to navigate and read because of lots of repeated lines (e.g.: adapters/mod.rs was 3k lines long). This is also in line with some adapters which already had their own modules (Flatten, FlatMap, Chain, Zip, Fuse).

This PR also makes Takes adapter fields private (I have no idea why they were pub(super) before).

r? @LukasKalbertodt

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2020
@WaffleLapkin
Copy link
Member Author

Also, I want to point out some strange things, I've found in sources:

  • RepeatWith and OnceWith derive debug whereas all other iterators which store functions implement it by hand to remove F: Debug bound
  • Repeat doesn't implement Copy, while OnceWith does

@bors
Copy link
Contributor

bors commented Oct 14, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@sdroege sdroege mentioned this pull request Oct 14, 2020
Closed
@WaffleLapkin
Copy link
Member Author

Rebased to fix conflict with #77870

@bors
Copy link
Contributor

bors commented Oct 19, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@crlf0710 crlf0710 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 6, 2020
@m-ou-se m-ou-se assigned m-ou-se and unassigned LukasKalbertodt Nov 12, 2020
@m-ou-se m-ou-se added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 12, 2020
@sdroege
Copy link
Contributor

sdroege commented Nov 20, 2020

What's the status here, is there anything I could help with to move it forward? It would help to get #77822 cleaned up.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 20, 2020

What's the status here

Lukas didn't have time to review this, so I reassigned this to me last week. I'll try to review this asap. Looks like there's a merge conflict right now though.

@sdroege
Copy link
Contributor

sdroege commented Nov 20, 2020

Yeah this will easily run into conflicts because it's moving around so much code :) Thanks for the fast reply, and in any case, if there's anything I could help with just let me know.

@WaffleLapkin
Split iterator adaptors into individual modules
773b73c 
This commit also makes fields of `Take` private. I have no idea why they
were `pub(super)` before.
@WaffleLapkin
Split iterator sources into different modules
66d6708
@WaffleLapkin
Merge uses in core::iter
b82a76a
@WaffleLapkin
Fix doc links in core::iter::sources
0dc187c
@WaffleLapkin
Fix UI tests
e0e906b 
Some UI tests started failing after moving iterator adapters to different modules.
@WaffleLapkin
Remove multiline uses
4612658
@WaffleLapkin
Copy link
Member Author

Oh, I somehow missed the conflict :o

Hope I didn't screw anything up, resolving conflicts with 5573a16, 8374c17, 84daccc and 8258cf2

@m-ou-se
Copy link
Member

m-ou-se commented Nov 21, 2020

@WaffleLapkin Thanks! I'll review this tomorrow. :)

@m-ou-se
Copy link
Member

m-ou-se commented Nov 22, 2020

I've verified the code in the split files is exactly identical to the code in the original files, except for imports and a few visibility specifiers.

Thanks @WaffleLapkin!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 22, 2020

📌 Commit 4612658 has been approved by m-ou-se

@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 Nov 22, 2020
@WaffleLapkin
Copy link
Member Author

Yay! Thanks for the review @m-ou-se!

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 22, 2020
@m-ou-se
Rollup merge of rust-lang#77697 - WaffleLapkin:iter_split_adaptors, r…
f7b4a0c 
…=m-ou-se

Split each iterator adapter and source into individual modules

This PR creates individual modules for each iterator adapter and iterator source.

This is done to enhance the readability of corresponding modules (`adapters/mod.rs` and `sources.rs`) which were hard to navigate and read because of lots of repeated lines (e.g.: `adapters/mod.rs` was 3k lines long). This is also in line with some adapters which already had their own modules (`Flatten`, `FlatMap`, `Chain`, `Zip`, `Fuse`).

This PR also makes `Take`s adapter fields private (I have no idea why they were `pub(super)` before).

r? `@LukasKalbertodt`
@m-ou-se m-ou-se mentioned this pull request Nov 22, 2020
Closed
@m-ou-se m-ou-se mentioned this pull request Nov 22, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2020
@bors
Auto merge of rust-lang#79319 - m-ou-se:rollup-d9n5viq, r=m-ou-se
32da90b 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#76941 (Add f{32,64}::is_subnormal)
 - rust-lang#77697 (Split each iterator adapter and source into individual modules)
 - rust-lang#78305 (Stabilize alloc::Layout const functions)
 - rust-lang#78608 (Stabilize refcell_take)
 - rust-lang#78793 (Clean up `StructuralEq` docs)
 - rust-lang#79267 (BTreeMap: address namespace conflicts)
 - rust-lang#79293 (Add test for eval order for a+=b)
 - rust-lang#79295 (BTreeMap: fix minor testing mistakes in rust-lang#78903)
 - rust-lang#79297 (BTreeMap: swap the names of NodeRef::new and Root::new_leaf)
 - rust-lang#79299 (Stabilise `then`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4407049 into rust-lang:master Nov 23, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 23, 2020
@lukaslueg lukaslueg mentioned this pull request Nov 23, 2020
Merged
@WaffleLapkin WaffleLapkin deleted the iter_split_adaptors branch December 1, 2020 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tesuji tesuji tesuji left review comments

Assignees

@m-ou-se m-ou-se

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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.50.0
Development

Successfully merging this pull request may close these issues.
9 participants
@WaffleLapkin @bors @sdroege @m-ou-se @tesuji @crlf0710 @rust-highfive @LukasKalbertodt @rustbot