make 'oc status' compatible with bare kube #1947
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k 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 |
openshift-ci
bot
added
the
approved
There was a problem hiding this comment.
Overall looks good to me and https://github.com/openshift/origin/blob/master/test/extended/cli/status.go likely catches any regression. Just dropped a comment.
| @@ -265,7 +265,7 @@ func (d *ProjectStatusDescriber) Describe(namespace, name string) (string, error | |||
| if allNamespaces { | |||
| fmt.Fprintf(out, describeAllProjectsOnServer(f, d.Server)) | |||
| } else { | |||
| fmt.Fprintf(out, describeProjectAndServer(f, project, d.Server)) | |||
There was a problem hiding this comment.
describeProjectAndServer is not used by anyone. Can we also remove it as well?.
There was a problem hiding this comment.
describeProjectAndServeris not used by anyone. Can we also remove it as well?.
Tidied up along with the project client.
deads2k
force-pushed
the
oc-project-aks
branch
from
256697a to
5d62039
Compare
| fakerouteclient "github.com/openshift/client-go/route/clientset/versioned/fake" | ||
| fakeroutev1client "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1/fake" | ||
| osgraph "github.com/openshift/oc/pkg/helpers/graph/genericgraph" | ||
| corev1 "k8s.io/api/core/v1" |
There was a problem hiding this comment.
Nit: there's still a visible value in keeping k8s.io and github.com package imports separated
|
/test verify-deps |
|
@deads2k: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
It is nice to see that we are dropping a dependency (i.e. |
Fixes an annoyance that I don't think has a purpose for remaining.