Skip to content

Conversation

jogru0
Copy link
Contributor

@jogru0 jogru0 commented Apr 8, 2025

Proposal: rust-lang/libs-team#435
Tracking Issue: #130711

This basically just reopens #130682 but squashed and with the new function and the feature gate renamed to next_index.

There are two questions I have already:

  • Shouldn't we add test coverage for that? I'm happy to provide some, but I might need a pointer to where these test would be.
    • Maybe I could actually also add a doctest?
  • For now, I just renamed the feature name in the unstable attribute to next_index, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?

@rustbot
Copy link
Collaborator

rustbot commented Apr 8, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 8, 2025
@jogru0 jogru0 marked this pull request as ready for review April 8, 2025 15:30
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

For now, I just renamed the feature name in the unstable attribute to next_index, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?

Renaming the feature (especially given we've not landed anything yet) seems fine.

@jogru0
Copy link
Contributor Author

jogru0 commented Apr 19, 2025

@Mark-Simulacrum I ended up adding a Doctest for the usual functionality of the feature, and a test in coretests to make sure it works correct for empty iterators.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 19, 2025

📌 Commit f121c7c has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@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 Apr 19, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2025
…enton

Rollup of 7 pull requests

Successful merges:

 - rust-lang#139042 (Do not remove trivial `SwitchInt` in analysis MIR)
 - rust-lang#139533 (add next_index to Enumerate)
 - rust-lang#139843 (Setup editor file associations for non-rs extensions)
 - rust-lang#140000 (skip llvm-config in autodiff check builds, when its unavailable)
 - rust-lang#140008 (Improve `clean_maybe_renamed_item` function code a bit)
 - rust-lang#140024 (Remove early exits from JumpThreading.)
 - rust-lang#140039 (Add option for stable backport poll)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 92adbd3 into rust-lang:master Apr 20, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 20, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2025
Rollup merge of rust-lang#139533 - jogru0:130711, r=Mark-Simulacrum

add next_index to Enumerate

Proposal: rust-lang/libs-team#435
Tracking Issue: rust-lang#130711

This basically just reopens rust-lang#130682 but squashed and with the new function and the feature gate renamed to `next_index.`

There are two questions I have already:
- Shouldn't we add test coverage for that? I'm happy to provide some, but I might need a pointer to where these test would be.
  - Maybe I could actually also add a doctest?
- For now, I just renamed the feature name in the unstable attribute to `next_index`, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 22, 2025
add next_index to Enumerate

Proposal: rust-lang/libs-team#435
Tracking Issue: rust-lang#130711

This basically just reopens rust-lang#130682 but squashed and with the new function and the feature gate renamed to `next_index.`

There are two questions I have already:
- Shouldn't we add test coverage for that? I'm happy to provide some, but I might need a pointer to where these test would be.
  - Maybe I could actually also add a doctest?
- For now, I just renamed the feature name in the unstable attribute to `next_index`, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

4 participants