Introduce Go std slices and maps lib #696
Conversation
k8s-ci-robot
added
kind/cleanup
k8s-ci-robot
added
the
size/L
✅ Deploy Preview for kubernetes-sigs-jobset canceled.
|
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
tenzen-y
force-pushed
the
introduce-go-std-lib
branch
from
79b6b3b to
4b1eddd
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
pkg/controllers/jobset_controller.go
Outdated
| @@ -731,7 +733,10 @@ func labelAndAnnotateObject(obj metav1.Object, js *jobset.JobSet, rjob *jobset.R | |||
| jobName := placement.GenJobName(js.Name, rjob.Name, jobIdx) | |||
|
|
|||
| // Set labels on the object. | |||
| labels := collections.CloneMap(obj.GetLabels()) | |||
| labels := maps.Clone(obj.GetLabels()) | |||
There was a problem hiding this comment.
Why do we need to clone the map here? similar question to the annotations clone below
There was a problem hiding this comment.
That is a good question.
TBH, I have the same question since we already set the constructed labels and annotations to object the below:
jobset/pkg/controllers/jobset_controller.go
Lines 786 to 787 in 4b1eddd
I thought this redundant clone came from historical reasons.
Anyway, let me remove these clones.
There was a problem hiding this comment.
Done.
If you want to keep these clones, let me know.
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
k8s-ci-robot
removed
the
lgtm
ahg-g
added
the
tide/merge-method-squash
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, tenzen-y 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 |
k8s-ci-robot
added
the
approved
| @@ -731,7 +733,7 @@ func labelAndAnnotateObject(obj metav1.Object, js *jobset.JobSet, rjob *jobset.R | |||
| jobName := placement.GenJobName(js.Name, rjob.Name, jobIdx) | |||
|
|
|||
| // Set labels on the object. | |||
| labels := collections.CloneMap(obj.GetLabels()) | |||
| labels := make(map[string]string) | |||
There was a problem hiding this comment.
this caused a regression, see #729
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The Go std library has the same functions as the collections pkg below.
So, I replace self-defined collections with Go std libs.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?