Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Renames the svcat --kube-context flag to --context (#1821) #1997

Merged

Conversation

Bubblemelon
Copy link
Contributor

@Bubblemelon Bubblemelon commented Apr 26, 2018

  • renamed strings with kube-context to context
  • this is to mimic kubectl's --context flag

Closes #1821

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 26, 2018
@carolynvs carolynvs self-requested a review April 27, 2018 00:20
Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Just some small changes, and then this will be all set! 👍

@@ -112,7 +112,7 @@ func buildRootCommand(cxt *command.Context) *cobra.Command {
},
}

cmd.PersistentFlags().StringVar(&opts.KubeContext, "kube-context", "", "name of the kube context to use")
cmd.PersistentFlags().StringVar(&opts.KubeContext, "context", "", "name of the kubeconfig context to use. This replaces the flag --kube-context")
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are not to v1.0 yet, we don't need to put "This replaces the flag..." in the help text.

"log-flush-frequency": {},
"logtostderr": {},
"match-server-version": {},
// "kube-context": {}, this now replaced with the flag "context"
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be deleted, nice catch!

@Bubblemelon
Copy link
Contributor Author

Hi @carolynvs ! Thank you so much for all your help 😄

I've made the changes you've requested and it's in this 6aae4d7 commit, as listed above on this thread.

Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you for fixing this! One step closer to v1.0! 🎉

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

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

says what it does, very straightforward

LGTM

@MHBauer MHBauer added the LGTM2 label Apr 27, 2018
@MHBauer MHBauer merged commit 226d847 into kubernetes-retired:master Apr 27, 2018
@Bubblemelon Bubblemelon deleted the rename-kubecontext-flag branch July 19, 2018 22:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants