Skip to content

Stabilize Iterator::step_by#51320

Merged
bors merged 1 commit into
rust-lang:masterfrom
tmccombs:step-by
Jun 10, 2018
Merged

Stabilize Iterator::step_by#51320
bors merged 1 commit into
rust-lang:masterfrom
tmccombs:step-by

Conversation

@tmccombs
Copy link
Copy Markdown
Contributor

@tmccombs tmccombs commented Jun 3, 2018

Fixes #27741

@rust-highfive
Copy link
Copy Markdown
Contributor

rust-highfive commented Jun 3, 2018

r? @Kimundi

(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 Jun 3, 2018
@kennytm kennytm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. relnotes Marks issues that should be documented in the release notes of the next release. labels Jun 3, 2018
@zslayton zslayton mentioned this pull request Jun 5, 2018
Closed
@GuillaumeGomez
Copy link
Copy Markdown
Member

GuillaumeGomez commented Jun 8, 2018

I'd really like to see this feature being stabilized as well.

cc @SimonSapin

@SimonSapin
Copy link
Copy Markdown
Contributor

SimonSapin commented Jun 8, 2018

r=me on the diff

@scottmcm Your comment at #27741 (comment) is not clear to me. Are you proposing to change the semantics Iterator::step_by? Are you opposed to stabilizing it with the current semantics? (That is, take this PR as-is.)

@SimonSapin
Copy link
Copy Markdown
Contributor

SimonSapin commented Jun 8, 2018

Also CC @rust-lang/libs

@scottmcm
Copy link
Copy Markdown
Member

scottmcm commented Jun 9, 2018

@SimonSapin Thanks for the ping! I think the design process for the method defined the right semantics -- (1..).step_by(2)giving 1, 3, 5, ... makes sense to me -- so I'm in favour of stabilizing as-is.

(The comment was about implementation awkwardness -- nth isn't a great fit for what StepBy needs -- but I'm confident that's surmountable within the existing semantics.)

@SimonSapin
Copy link
Copy Markdown
Contributor

SimonSapin commented Jun 9, 2018

Thanks for confirming!

@bors r+

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jun 9, 2018

📌 Commit 72e17b8 has been approved by SimonSapin

@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 Jun 9, 2018
@bors
Copy link
Copy Markdown
Collaborator

bors commented Jun 10, 2018

⌛ Testing commit 72e17b8 with merge b4d836c...

bors added a commit that referenced this pull request Jun 10, 2018
@bors
Auto merge of #51320 - tmccombs:step-by, r=SimonSapin
b4d836c 
Stabilize Iterator::step_by

Fixes #27741
@bors
Copy link
Copy Markdown
Collaborator

bors commented Jun 10, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: SimonSapin
Pushing b4d836c to master...

@bors bors merged commit 72e17b8 into rust-lang:master Jun 10, 2018
@tmccombs tmccombs deleted the step-by branch August 16, 2018 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Kimundi Kimundi

Labels

relnotes Marks issues that should be documented in the release notes of the next release. 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@tmccombs @rust-highfive @GuillaumeGomez @SimonSapin @scottmcm @bors @kennytm @Kimundi