craigcondit commented on PR #422:
URL: https://github.com/apache/yunikorn-k8shim/pull/422#issuecomment-1138828374
> @craigcondit Thanks for your review. I would like some clarifications.
Need to open another jira to put all the `zap.String("", var.String())` to
`zap.Stringer("", var)` ? then I found
>
> ```
> func beforeHook(event TaskEventType) string {
> return fmt.Sprintf("before_%s", event.String())
> }
> ```
>
> .String() can be removed :
>
> ```
> func beforeHook(event TaskEventType) string {
> return fmt.Sprintf("before_%s", event)
> }
> ```
Logging APIs should avoid `.String()` as the `String()` call will be
evaluated eagerly, while the log message may not be printed at all if the log
level isn't high enough. Therefore, avoiding `.String()` saves cycles.
The `fmt.Sprintf()` calls are less critical as they will be evaluated
unconditionally.
I don't think we need a separate cleanup PR to do this in the entire code
base; it's not that critical. But, we can clean things up as we refactor in
other places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
