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

Stabilize slice_group_by #117678

Merged
merged 3 commits into from
Jan 26, 2024
Merged

Stabilize slice_group_by #117678

merged 3 commits into from
Jan 26, 2024

Conversation

niklasf
Copy link
Contributor

@niklasf niklasf commented Nov 7, 2023

Renamed "group by" to "chunk by" a per #80552.

Newly stable items:

  • core::slice::ChunkBy
  • core::slice::ChunkByMut
  • [T]::chunk
  • [T]::chunk_by

Closes #80552.

@niklasf
Stabilize slice_group_by
0bccdb3 
Renamed "group by" to "chunk by" a per rust-lang#80552.

Newly stable items:

* `core::slice::ChunkBy`
* `core::slice::ChunkByMut`
* `[T]::chunk`
* `[T]::chunk_by`

Closes rust-lang#80552.
@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2023

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

@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 Nov 7, 2023
@niklasf
Copy link
Contributor Author

niklasf commented Nov 7, 2023

@rustbot modify labels: +T-libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 7, 2023
@niklasf
Copy link
Contributor Author

niklasf commented Nov 7, 2023

Note: I renamed the items as discussed during the FCP, but I was not sure if the name in #[stable(feature = ...)] is also supposed to be renamed.

@WaffleLapkin
Copy link
Member

@niklasf feature name generally stays the same, so that it can be searched.

marcospb19
marcospb19 reviewed Nov 8, 2023
View reviewed changes
Copy link
Contributor

@marcospb19 marcospb19 left a comment

Choose a reason for hiding this comment

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

Doesn't this renaming breaks code at

rust/src/tools/rust-analyzer/crates/hir/src/source_analyzer.rs

Lines 669 to 670 in 755629f

for (macro_id, mut helpers) in
helpers.iter().group_by(|(_, macro_id, ..)| macro_id).into_iter()
and the rest of the code in this repo that uses group_by?

@niklasf
Copy link
Contributor Author

niklasf commented Nov 8, 2023 •
edited
Loading

That is Itertools::group_by which works on iterators, while [T]::chunk_by works on slices.

@jdahlstrom
Copy link

jdahlstrom commented Nov 17, 2023 •
edited
Loading

/// The predicate is called on two elements following themselves,
/// it means the predicate is called on slice[0] and slice[1]
/// then on slice[1] and slice[2] and so on.
...
pub fn chunk_by(&self, pred: F) -> ChunkBy<'_, T, F>

This paragraph could probably be rewritten to make more sense before stabilization? Something like "called for every pair of consecutive elements, meaning that it is called…".

@niklasf
Copy link
Contributor Author

niklasf commented Nov 17, 2023

Fixed the wording (c3a8237).

marcospb19
marcospb19 approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@marcospb19 marcospb19 left a comment

Choose a reason for hiding this comment

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

This LGTM

@tobymurray tobymurray mentioned this pull request Nov 22, 2023
Closed
@dtolnay dtolnay assigned dtolnay and unassigned thomcc Jan 26, 2024
dtolnay
dtolnay approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you!

@dtolnay
Copy link
Member

dtolnay commented Jan 26, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2024

📌 Commit c3a8237 has been approved by dtolnay

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 Jan 26, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2024
@matthiaskrgr
Rollup merge of rust-lang#117678 - niklasf:stabilize-slice_group_by, …
7749e49 
…r=dtolnay

Stabilize `slice_group_by`

Renamed "group by" to "chunk by" a per rust-lang#80552.

Newly stable items:

* `core::slice::ChunkBy`
* `core::slice::ChunkByMut`
* `[T]::chunk`
* `[T]::chunk_by`

Closes rust-lang#80552.
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 26, 2024
Closed
@matthiaskrgr
Copy link
Member

@bors r-
needs rebase

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 26, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jan 26, 2024

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@dtolnay
Copy link
Member

dtolnay commented Jan 26, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2024

📌 Commit 97a720b has been approved by dtolnay

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 26, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 26, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
@bors
Auto merge of rust-lang#120375 - matthiaskrgr:rollup-ueakvms, r=matth…
e7bbe8c 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#117420 (Make `#![allow_internal_unstable(..)]` work with `stmt_expr_attributes`)
 - rust-lang#117678 (Stabilize `slice_group_by`)
 - rust-lang#119917 (Remove special-case handling of `vec.split_off(0)`)
 - rust-lang#120117 (Update `std::io::Error::downcast` return type)
 - rust-lang#120329 (RFC 3349 precursors)
 - rust-lang#120339 (privacy: Refactor top-level visiting in `NamePrivacyVisitor`)
 - rust-lang#120345 (Clippy subtree update)
 - rust-lang#120360 (Don't fire `OPAQUE_HIDDEN_INFERRED_BOUND` on sized return of AFIT)
 - rust-lang#120372 (Fix outdated comment on Box)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a5b60c9 into rust-lang:master Jan 26, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
@rust-timer
Unrolled build for rust-lang#117678
48e1307 
Rollup merge of rust-lang#117678 - niklasf:stabilize-slice_group_by, r=dtolnay

Stabilize `slice_group_by`

Renamed "group by" to "chunk by" a per rust-lang#80552.

Newly stable items:

* `core::slice::ChunkBy`
* `core::slice::ChunkByMut`
* `[T]::chunk`
* `[T]::chunk_by`

Closes rust-lang#80552.
@cuviper cuviper mentioned this pull request Jan 26, 2024
Merged
@femshima femshima mentioned this pull request Feb 20, 2024
Merged
@slanterns slanterns mentioned this pull request Mar 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcospb19 marcospb19 marcospb19 approved these changes

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

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

Successfully merging this pull request may close these issues.

Tracking Issue for the GroupBy and GroupByMut iterators
9 participants
@niklasf @rustbot @WaffleLapkin @jdahlstrom @dtolnay @bors @matthiaskrgr @marcospb19 @thomcc