Skip to content
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 misleading and ugly kubectl parameters #9193

Closed
wants to merge 1 commit into from

Conversation

afbjorklund
Copy link
Collaborator

@afbjorklund afbjorklund commented Sep 6, 2020

Chances are users want something more than "get pods"

💡 kubectl not found. If you need it, try: 'minikube kubectl -- get pods -A'

Running the bare command will show some usage info

kubectl controls the Kubernetes cluster manager.

 Find more information at: https://kubernetes.io/docs/reference/kubectl/overview/
...

It looks a bit ugly in the new screen shot, in my opinion

Also helps a bit on the way to making an alias: #7289

Chances are users want something more than "get pods"

Running the bare command will show some usage info
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 6, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund

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

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 6, 2020
@@ -413,7 +413,7 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string, machineName st

path, err := exec.LookPath("kubectl")
if err != nil {
out.T(style.Tip, "kubectl not found. If you need it, try: 'minikube kubectl -- get pods -A'")
out.T(style.Tip, "kubectl not found. If you need it, try: 'minikube kubectl --'")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, given the strange syntax, I don't feel like this is obvious enough. I wanted to give an example that a user is likely to understand without consulting additional documentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe 'minikube kubectl -- help'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beauty, as they say, is in the eye of the beholder. Maybe I should have used a different word (than ugly).

Did you know that kubectl, kubectl --help and kubectl help all have the same output, by the way ?

I like "help" as an improvement (over "get"), but it wasn't really what I meant.

Maybe I will just close this PR, it was just the new screenshot that caused it.

screenshot

It looked a bit like an error, while it is perfectly normal. But maybe we want to avoid "other solutions" like:


Command 'kubectl' not found, but can be installed with:

sudo snap install kubectl

It just seemed weird to give an example command, rather than show how to "install" it (e.g. as an alias).

@afbjorklund afbjorklund closed this Sep 9, 2020
@afbjorklund
Copy link
Collaborator Author

Here is how it looked in bash, for reference.

minikube-start-ubuntu-virtualbox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants