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

Add <NAME> arg to help output #1904

Closed
wants to merge 1 commit into from
Closed

Conversation

heiko-braun
Copy link
Contributor

What issue type does this pull request address? (keep at least one, remove the others)
/kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves #
ENG-4070

Copy link

netlify bot commented Jul 4, 2024

Deploy Preview for vcluster-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit e580ce9
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/6686ac7a26f89e00081d070d

@@ -33,7 +33,7 @@ vcluster platform add vcluster my-vcluster --namespace vcluster-my-vcluster --pr
`

addCmd := &cobra.Command{
Use: "vcluster",
Use: "vcluster VCLUSTER_NAME",
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also use this util func that we already use in some places (probably not consistent over the whole code base): https://github.com/loft-sh/vcluster/blob/main/cmd/vclusterctl/cmd/create.go#L35

@@ -33,7 +33,7 @@ vcluster platform add vcluster my-vcluster --namespace vcluster-my-vcluster --pr
`

addCmd := &cobra.Command{
Use: "vcluster",
Use: "vcluster VCLUSTER_NAME",
Short: "Adds an existing vCluster to the vCluster platform",
Long: description,
Args: cobra.ExactArgs(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -50,7 +50,7 @@ func NewClusterCmd(globalFlags *flags.GlobalFlags) *cobra.Command {
}

c := &cobra.Command{
Use: "cluster",
Use: "cluster CLUSTER_NAME",
Copy link
Contributor

Choose a reason for hiding this comment

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

If we go the util route we might want to add a Cluster entry there as well!?

Copy link
Member

@FabianKramm FabianKramm left a comment

Choose a reason for hiding this comment

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

We need to be careful here as Use is also used to build the command itself

@heiko-braun
Copy link
Contributor Author

Can you elaborate @FabianKramm ?

@heiko-braun heiko-braun closed this Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants