馃尡 ClusterCacheTracker: drop unused Log field #6318
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
lgtm, technically incompatible change? |
|
Note: we can do breaking changes in golang between minors, it is just a matter to document them (only API types and CLI have different guarantees) |
|
Given that this type is likely to be set outside the codebase I think deprecation and removal at the next version update would be better for consumers, even if outright removal is allowed. |
|
Yup looks like based on our guidelines I have to deprecate it in a minor release first: https://github.com/kubernetes-sigs/cluster-api/blob/88b5a8edc855775a2545b61d30b1470c6b21f26d/CONTRIBUTING.md#codebase-and-go-modules I'll change the PR to still drop all usages but only deprecate the field in the struct. |
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
sbueringer
force-pushed
the
pr-drop-unused-logger
branch
from
682af14
to
4ce8299
Compare
| @@ -44,6 +45,7 @@ type ClusterCacheReconciler struct { | |||
|
|
|||
| func (r *ClusterCacheReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { | |||
| err := ctrl.NewControllerManagedBy(mgr). | |||
| Named("remote/clustercache"). | |||
There was a problem hiding this comment.
@fabriziopandini I set the name here.
This will lead to a "controller": "remote/clustercache" k/v pair.
Before this PR the logs from this controller had "controller": "cluster" (same as the cluster controller).
Note: This info is based on controller-runtime main. The current v0.11 release will only use the name as prefix for messages ("controller/cluster: <msg>").
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre 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
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
What this PR does / why we need it:
As we are always using the logger from context we don't have to set the Log field anymore, so let's drop it.
Note: before this PR the fields was written a few times, but never read.
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 #