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

all: drop x/exp direct dependency #30558

Merged
merged 5 commits into from
Feb 27, 2025
Merged

all: drop x/exp direct dependency #30558

merged 5 commits into from
Feb 27, 2025

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Oct 8, 2024

This is a not-particularly-important "cleanliness" PR. It removes the last remnants of the x/exp package, where we used the maps.Keys function.

The original returned the keys in a slice, but when it became 'native' the signature changed to return an iterator, so the new idiom is slices.Collect(maps.Keys(theMap)), unless of course the raw iterator can be used instead.

In some cases, where we previously collect into slice and then sort, we can now instead do slices.SortXX on the iterator instead, making the code a bit more concise.

This PR might be slighly less optimal, because the original x/exp implementation allocated the slice at the correct size off the bat, which I suppose the new code won't.

Putting it up for discussion.

@holiman
Copy link
Contributor Author

holiman commented Oct 8, 2024

Ah, the slices.SortedFunc requires 1.23. Guess this PR will have to wait

@holiman
Copy link
Contributor Author

holiman commented Feb 12, 2025

Now that go 1.24 has been released, this can be merged as soon as support for 1.22 is dropped. Rebased on latest master.

@holiman
Copy link
Contributor Author

holiman commented Feb 13, 2025

Depends on #31159

fjl
fjl previously approved these changes Feb 27, 2025
@fjl fjl added this to the 1.15.4 milestone Feb 27, 2025
@fjl fjl merged commit 767c202 into ethereum:master Feb 27, 2025
3 of 4 checks passed
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.

2 participants