-
Couldn't load subscription status.
- Fork 42
Updated component-base/logs instead of old logging style #615
Conversation
|
Welcome @Vasubabu! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll want to try compiling this locally first before pushing. There's definitely an extra definition of pflag here and I imagine a make generate needs to be run too.
main.go
Outdated
|
|
||
| cliflag "k8s.io/component-base/cli/flag" | ||
| "k8s.io/component-base/logs" | ||
| v1 "k8s.io/component-base/logs/api/v1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why we don't need the _ "k8s.io/component-base/logs/json/register" line they had in the original code suggestion?
Also, can we call this something other than v1. I'd prefer something like logsv1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
7eb5286 to
cccdca7
Compare
|
For reference, here's the upstream change that started normalizing flag input: sbueringer/cluster-api@21740cb Seems like a good thing to do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change line 106 from
ctrl.SetLogger(klogr.New())
to
// klog.Background will automatically use the right logger.
ctrl.SetLogger(klog.Background())
This is per another upstream change that they got in later.
|
/approve |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cprivitere, vasubabu 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 |
What this PR does / why we need it:
Updated component-base/logs instead of old logging style.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes # #546