Change sort to slices package#503
Conversation
Signed-off-by: dongjiang <dongjiang1989@126.com>
k8s-ci-robot
added
cncf-cla: yes
✅ Deploy Preview for agent-sandbox canceled.
|
|
/assign @vicentefb |
There was a problem hiding this comment.
leaving a note why I think this is a positive change; https://pkg.go.dev/sort#Slice specifically states:
Note: in many situations, the newer [slices.SortFunc](https://pkg.go.dev/slices#SortFunc) function is more ergonomic and runs faster.
/lgtm
k8s-ci-robot
added
the
lgtm
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
|
/assign |
| sort.Slice(oldStatus.Conditions, func(i, j int) bool { | ||
| return oldStatus.Conditions[i].Type < oldStatus.Conditions[j].Type | ||
| slices.SortFunc(oldStatus.Conditions, func(a, b metav1.Condition) int { | ||
| if a.Type < b.Type { |
There was a problem hiding this comment.
Nit: can we use cmp.Compare(a.Type, b.Type)? It makes the new logic easier to follow IMO
| sort.Slice(claim.Status.Conditions, func(i, j int) bool { | ||
| return claim.Status.Conditions[i].Type < claim.Status.Conditions[j].Type | ||
| slices.SortFunc(claim.Status.Conditions, func(a, b metav1.Condition) int { | ||
| if a.Type < b.Type { |
There was a problem hiding this comment.
(Again - will cmp.Compare work?)
| slices.SortFunc(candidates, func(a, b *v1alpha1.Sandbox) int { | ||
| aReady := isSandboxReady(a) | ||
| bReady := isSandboxReady(b) | ||
| if aReady != bReady { |
There was a problem hiding this comment.
I like this; it's much clearer (with the comments!)
|
I had a nit about using cmp.Compare, but it's such a small nit I don't want to hold up the PR /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dongjiang1989, justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
6 participants
Replace the
sortpackage with theslicespackage to simplify the code.