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.

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

Deprecate tree_fold1 for tree_reduce #895

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented Mar 7, 2024

I think tree_fold1 should have been deprecated for tree_reduce at the same time fold1 was deprecated for reduce.

With this deprecation, we will be able to remove tree_fold1 and fold1 at the same time and close that chapter once and for all.

@scottmcm Do you have an objection on this?

PS: I wish I had thought of this but no, thanks reddit.

Copy link
Member

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me.

@scottmcm
Copy link
Contributor

Nope, no objections to me! Being consistent is good.

(Feel free to rename however you think is best. Could also consider reduce_tree or something for improved sorting, though it doesn't read quite as nicely so up to you.)

@jswrenn jswrenn added this to the next milestone Mar 14, 2024
@Philippe-Cholet
Copy link
Member Author

I'm okay with both. @jswrenn Feel free to merge or ask me to change the name.

@jswrenn
Copy link
Member

jswrenn commented Mar 14, 2024

Let's stick with tree_reduce. We don't have any other reduce* methods, so sorting doesn't matter too much for us. If the standard library decides to stabilize such a method, the libs team might want to revisit the name.

@jswrenn jswrenn added this pull request to the merge queue Mar 14, 2024
Merged via the queue into rust-itertools:master with commit 45c5dec Mar 14, 2024
11 checks passed
@Philippe-Cholet Philippe-Cholet deleted the deprecate-tree_fold1 branch March 14, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants