-
Notifications
You must be signed in to change notification settings - Fork 39.9k
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
Remove kubectl's dependence on schema file in pkg/api/validation. #46317
Conversation
/assign @pwittrock |
/assign @lavalamp Need API owner's approval. Please approve to unblock kubectl isolation work :) |
looks like a test is annoyed by a moved data file. will fix tomorrow, but meantime feel free to approve given that the merge won't happen till this is fixed. |
18c3d72
to
d4fa2ba
Compare
ca728c9
to
b7e015b
Compare
FYI, there is a test failure |
71c95a2
to
9a35c87
Compare
@k8s-bot pull-kubernetes-node-e2e test this |
@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this |
@k8s-bot pull-kubernetes-node-e2e test this |
/lgtm |
/approve |
The kubectl decoupling project (kubernetes#598) requires many BUILD edits. Even relatively simple PR's involve many OWNER files, e.g. kubernetes#46317 involves five. We plan to script-generate some PRs, and those may involve _hundreds_ of BUILD files. This project will take many PRs, and collecting all approvals for each will be very time consuming.
/assign @mikedanese /assign @ixdy /assign @erictune |
/assign @apelisse Antoine, PTAL, as this overlaps w/ your openapi work a bit. |
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.
All the lint-detected changes are super welcome.
Thanks for working on that, maybe I'd have liked a better separation of these multiples changes, but whatever
pkg/api/errors/BUILD
Outdated
@@ -5,11 +5,6 @@ load( | |||
"go_library", | |||
) | |||
|
|||
go_library( |
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.
Yeah, it's unfortunate you have to do that...
@@ -32,6 +32,7 @@ import ( | |||
utilfeature "k8s.io/apiserver/pkg/util/feature" | |||
"k8s.io/kubernetes/pkg/api" | |||
"k8s.io/kubernetes/pkg/api/helper" | |||
_ "k8s.io/kubernetes/pkg/api/testapi" |
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.
OK I think that answers my question from this morning.
/lgtm |
/test pull-kubernetes-unit |
Needs rebase :-/ |
19f532d
to
0df066c
Compare
@monopole Feel free to self-lgtm after rebase. |
0df066c
to
a920a2e
Compare
/lgtm |
@monopole: you cannot LGTM your own PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/lgtm |
**What this PR does / why we need it**: Makes functions in validation/schema.go private to kubectl, further isolating kubectl. **Which issue this PR fixes** Part of a series of PRs to address kubernetes/community#598 **Release note**: ```release-note NONE ```
a920a2e
to
dbc22ad
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apelisse, ixdy, mengqiy, mikedanese, monopole, pwittrock Associated issue: 598 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
What this PR does / why we need it:
Makes functions in validation/schema.go private to kubectl,
further isolating kubectl. This move revealed a "hidden" dependence
(a dependence not expressed in a BUILD or make file) from a feature
level test in /hack/make-rules on a kubectl test data file. So this
PR also adds some BUILD rules around the relevant hack targets, to make the
dependence official. A later PR will move the kubectl aspect of this "hack"
test into a kubectl test directory. Leaving it in place for now after establishing
and "official" dependency, since moving the test beyond PR scope. The
test also depends on a small sh file in //cluster, which makes no sense.
Which issue this PR fixes
Part of a series of PRs to address kubernetes/community#598
Release note: