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

cmd/devp2p/internal/ethtest: using slices.SortFunc to simplify the code #31012

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dashangcun
Copy link

@dashangcun dashangcun commented Jan 9, 2025

Using slices.SortFunc to simplify the code.

lightclient
lightclient previously approved these changes Jan 9, 2025
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

SGTM

@fjl
Copy link
Contributor

fjl commented Jan 9, 2025

This type can probably be deleted, and its use replaced by slices.SortFunc

@fjl fjl changed the title refactor: rewrite swap to simplify code cmd/devp2p/internal/ethtest: rewrite swap to simplify code Jan 9, 2025
@gitglorythegreat
Copy link
Contributor

If this kind of code style change is worthwhile, would you reconsider #30957?

@fjl
Copy link
Contributor

fjl commented Jan 10, 2025

In general, we do not care too much about improving small details like this. If the code works, we think it's fine. And it will be rewritten or deleted at some point anyway.

I suggested to remove type Addresses instead of fixing a style issue in the function because the type only has one use, and this use can be removed by using slices.SortFunc instead.

@dashangcun
Copy link
Author

dashangcun commented Jan 10, 2025

In general, we do not care too much about improving small details like this. If the code works, we think it's fine. And it will be rewritten or deleted at some point anyway.

I suggested to remove type Addresses instead of fixing a style issue in the function because the type only has one use, and this use can be removed by using slices.SortFunc instead.

Thanks for your reply. I am happy to make such changes.

The code has been optimized using slices.SortFunc. Please review again.

@dashangcun dashangcun changed the title cmd/devp2p/internal/ethtest: rewrite swap to simplify code cmd/devp2p/internal/ethtest: using slices.SortFunc to simplify the code Jan 10, 2025
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.

4 participants