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

Move versioned generators into their own package #68441

Merged
merged 5 commits into from Oct 17, 2018

Conversation

@smarterclayton
Contributor

smarterclayton commented Sep 9, 2018

pkg/kubectl has a lot of files. Move everything generator related
into pkg/kubectl/generate (generic) or pkg/kubectl/generate/versioned
(type specific).

Move the DescriberFn and GeneratorFn out of kubectl/cmd/util and into
the respective versioned packages, along with tests.

@kubernetes/sig-cli-bugs

What this PR does / why we need it:

Continue to clean up pkg/kubectl, separate out implementation and interfaces

Special notes for your reviewer:
Release note:

NONE
@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented Sep 9, 2018

@deads2k @soltysh hit this while I was trying to split binaries out of a command - I realized that kubectl/cmd/util still pulls in all internal types.

@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented Sep 9, 2018

Also cuts another dependency like
#68423 on the core kubernetes repo and the docker engine apis (kubectl wants neither)

@k8s-ci-robot k8s-ci-robot added size/XXL and removed size/XL labels Sep 13, 2018

@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented Sep 19, 2018

@soltysh when you have chance if you could review

@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented Sep 20, 2018

/retest

@dims

This comment has been minimized.

Member

dims commented Sep 24, 2018

/assign @soltysh

@soltysh

@smarterclayton sorry for the delay this all lgtm, would you mind rebasing it and once you do feel free to self-tag it so that it lands fast. I don't want to hold you back on this.

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Oct 16, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented Oct 16, 2018

/retest

smarterclayton added some commits Sep 9, 2018

Move versioned generators into their own package
pkg/kubectl has a lot of files. Move everything generator related
into pkg/kubectl/generate (generic) or pkg/kubectl/generate/versioned
(type specific).

Move the DescriberFn and GeneratorFn out of kubectl/cmd/util and into
the respective versioned packages, along with tests.
Cut a dependency between kubectl and the rest of the repo
The types referenced in credentialprovider are part of a long term api
and will not change, and kubectl doesn't need to take a dependency on this
package in order to do minimal validation here.
Remove defaulting from test that uses kubectl.Scheme
The dependency on printers/internalversion was causing kubectl/scheme
to have defaulters registered, which should not be the case. Remove
defaults from the test.

@k8s-ci-robot k8s-ci-robot added the lgtm label Oct 17, 2018

@soltysh

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit 0d6f6a6 into kubernetes:master Oct 17, 2018

18 checks passed

cla/linuxfoundation smarterclayton authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment