Use Go standard library slices package instead of k8s.io/utils/strings/slices #126715
Conversation
…s/slices The package was introduced in Go 1.21 and is already in use in the k8s code base.
|
Hi @tklauser. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
| @@ -313,7 +313,7 @@ func (r Requirement) Equal(x Requirement) bool { | |||
| if r.operator != x.operator { | |||
| return false | |||
| } | |||
| return stringslices.Equal(r.strValues, x.strValues) | |||
| return slices.Equal(r.strValues, x.strValues) | |||
There was a problem hiding this comment.
what's the performance difference between the specific implementation and the generic? are we planning to remove the stringslices package?
There was a problem hiding this comment.
We committed to supporting k8s.io/utils - but we could^Wshould deprecate these, or reimplement them in terms of slices, or BOTH
|
/cc @thockin |
There was a problem hiding this comment.
It would be awesome to couple this with a PR against k8s.io/utils to mark these functions and maybe this whole package as deprecated, and reimplement them in terms of std slices:
e.g.
// Equal reports whether two slices are equal
// Deprecated: use slices.Equal instead
var Equal = slices.Equal
/lgtm
/approve
| @@ -313,7 +313,7 @@ func (r Requirement) Equal(x Requirement) bool { | |||
| if r.operator != x.operator { | |||
| return false | |||
| } | |||
| return stringslices.Equal(r.strValues, x.strValues) | |||
| return slices.Equal(r.strValues, x.strValues) | |||
There was a problem hiding this comment.
We committed to supporting k8s.io/utils - but we could^Wshould deprecate these, or reimplement them in terms of slices, or BOTH
|
LGTM label has been added. Git tree hash: 5d147bab9b7f81bf13e6f297fef9a9e124aa613a
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thockin, tklauser The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
As suggested by Tim in [1], deprecate the functions in the k8s.io/utils/strings/slices package that are already covered by generic functions in the Go standard library slices package. Also reimplement them in terms of slices functions. [1] kubernetes/kubernetes#126715 (review)
Thanks for the review and the suggestion. I've opened the corresponding |
As suggested by Tim in [1], deprecate the functions in the k8s.io/utils/strings/slices package that are already covered by generic functions in the Go standard library slices package. Also reimplement them in terms of slices functions. [1] kubernetes/kubernetes#126715 (review)
The package was introduced in Go 1.21 and is already in use in the k8s code base.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: