-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
client-go: don't import client auth provider packages #41532
client-go: don't import client auth provider packages #41532
Conversation
Removes 23 package dependencies (not projects) from
|
Proposed the same change before. Lgtm, but let somebody from sig-auth take a look. |
Partially adress kubernetes/client-go#95. |
Thanks @ericchiang. |
/lgtm |
@@ -35,7 +35,6 @@ import ( | |||
v1alpha1rbac "k8s.io/client-go/kubernetes/typed/rbac/v1alpha1" | |||
v1beta1rbac "k8s.io/client-go/kubernetes/typed/rbac/v1beta1" | |||
v1beta1storage "k8s.io/client-go/kubernetes/typed/storage/v1beta1" | |||
_ "k8s.io/client-go/plugin/pkg/client/auth" |
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 file is generated, compare
imports = append(imports, "_ \"k8s.io/client-go/plugin/pkg/client/auth\"") |
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.
I.e. please change the generator.
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.
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.
Nice catch!
8908b65
to
99e989e
Compare
/lgtm |
@lavalamp could you approve the change? Thanks. |
99e989e
to
163f37f
Compare
rebased |
@lavalamp could you approve the change? Thanks. |
/approve
…On Tue, Feb 21, 2017 at 12:47 PM, Chao Xu ***@***.***> wrote:
@lavalamp <https://github.com/lavalamp> could you approve the change?
Thanks.
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#41532 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p80WTuC6cUF_7NLciwh0UH7PXnR5ks5reyMtgaJpZM4MClXd>
.
|
[APPROVALNOTIFIER] This PR is APPROVED The following people have approved this PR: caesarxuchao, ericchiang, smarterclayton Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
@k8s-bot cvm gce e2e test this |
Automatic merge from submit-queue (batch tested with PRs 41349, 41532, 41256, 41587, 41657) |
Both of these auth providers are useful for kubectl but not so much for everyone importing client-go. Let users optionally import them (example [0]) and reduce the overall number of imports that client-go requires.
Quick grep seems to imply it wont import it after.
closes kubernetes/client-go#49
updates kubernetes/client-go#79 (removes cloud.google.com/go import)
cc @kubernetes/sig-api-machinery-pr-reviews @kubernetes/sig-auth-pr-reviews
[0] https://github.com/kubernetes/client-go/blob/8b466d64c5da37e0d3985b907d812e1f58cb41cb/examples/third-party-resources/main.go#L34-L35