-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: move FieldManager to more available package #397
refactor: move FieldManager to more available package #397
Conversation
) | ||
|
||
const ( | ||
FieldManager = client.FieldOwner("trust-manager") |
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.
This constant seems a bit "misplaced", but I didn't want to create a new Go-file for just this...
6a273e4
to
bdc202e
Compare
Signed-off-by: Erik Godding Boye <[email protected]>
bdc202e
to
d0647cb
Compare
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inteon 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 |
With some additional improvements that made sense when doing this. The main motivation is to reduce the size of #378, as I need the
FieldManager
constant available in a new package without getting circular dependencies.The package name
ssa_client
annoys me a bit. 😉 I think the name should be justclient
. I was tempted to change this, but let's leave it for now - as it will require us to resolve package name import conflicts. It should be possible to do by creating type aliases in ourclient
package. I've seen that c/r does this a lot.