Simplify buildLaunchTemplateTask() part one #11452
Conversation
k8s-ci-robot
added
kind/cleanup
| HTTPPutResponseHopLimit: lc.HTTPPutResponseHopLimit, | ||
| Name: fi.String(name), | ||
| Lifecycle: b.Lifecycle, | ||
| CPUCredits: fi.String(fi.StringValue(ig.Spec.CPUCredits)), |
There was a problem hiding this comment.
Could we keep this similar to others, without String(StringValue())?
| CPUCredits: fi.String(fi.StringValue(ig.Spec.CPUCredits)), | |
| CPUCredits: fi.String(""), |
There was a problem hiding this comment.
The previous code turns nil into *"". String(StringValue()) is the simplest way to do that. The suggested replacement ignores the value of ig.Spec.CPUCredits.
There was a problem hiding this comment.
Meant to add below something like, which may be a little easier to read:
if ig.Spec.CPUCredits != nil {
lt.CPUCredits = ig.Spec.CPUCredits
}
There was a problem hiding this comment.
Anyway, it's just a nit, if you like it more this way, I am ok with that too.
There was a problem hiding this comment.
I find splitting the logic into two places and coding it with a verbose if statement to be harder to read, but then I find a lot of idiomatic Go to be annoyingly verbose.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman 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
|
/retest |
/kind cleanup