-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Use Go standard library slices package more extensively #34796
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for my CODEOWNERs, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files owned by teams I am on seem good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for envoy related files. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm for my codeowners
a9eaab3
to
807bfc4
Compare
/test Rebased to resolve merge conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BGP changes looks good. Thank you 🙏
The former is available since Go 1.21 and the latter is merely a wrapper around it when using Go ≥ 1.21. Use the standard library package directly. Signed-off-by: Tobias Klauser <tobias@cilium.io>
Doing this because the slices functions are slightly faster and easier to use. It also removes one dependency layer. This requires changing SortedUniqueFunc to take the less function as a func(a, b T) instead of func(i, j int) which is more idiomatic anyway. Signed-off-by: Tobias Klauser <tobias@cilium.io>
This allows GC to reclaim memory for these elements. Signed-off-by: Tobias Klauser <tobias@cilium.io>
807bfc4
to
9a0cfa4
Compare
/test Rebased to resolve conflicts, once again. |
In recent Go versions sort.{Ints,Strings} is already implemented in terms of slices.Sort. This change removes that indirection and a dependency layer. Signed-off-by: Tobias Klauser <tobias@cilium.io>
Use the generic Contains* functions from the slices Go standard library package instead of open-coding them across the code base. This also allows simplifying some code. Signed-off-by: Tobias Klauser <tobias@cilium.io>
9a0cfa4
to
13413c4
Compare
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice cleanup, lgtm for Hubble!
❤️ the simplification! Thanks @tklauser! |
Make use of some newly added functionality added in Go 1.23.
See commit messages for details.