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

chore(cli): improve flag help descriptions to show allowed values instead of type names #5067

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

h3nryc0ding
Copy link
Contributor

@h3nryc0ding h3nryc0ding commented Nov 10, 2024

Closes #5066

…type names

Signed-off-by: h3nryc0ding <hr.richterhenry@gmail.com>
Signed-off-by: h3nryc0ding <hr.richterhenry@gmail.com>
Signed-off-by: h3nryc0ding <hr.richterhenry@gmail.com>
Copy link
Contributor Author

@h3nryc0ding h3nryc0ding left a comment

Choose a reason for hiding this comment

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

I've found some places where I'm unsure if shortening the descriptions, as done elsewhere, is a good idea. I'd really appreciate any feedback on this.

"where kind must be one of: (%s)",
strings.Join(supportedHelmChartSourceKinds, ", "),
)
return "source that contains the chart in the format '<kind>/<name>.<namespace>'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here

Copy link
Contributor

@darkowlzz darkowlzz Nov 15, 2024

Choose a reason for hiding this comment

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

At present (not with this change), this looks like

--source helmChartSource         source that contains the chart in the format '<kind>/<name>.<namespace>', where kind must be one of: (HelmRepository, GitRepository, Bucket)

I think the following may be better

--source <kind>/<name>.<namespace>         source that contains the chart, source kind must be one of: (HelmRepository, GitRepository, Bucket)

I'm afraid that something like HelmRepository|GitRepository|Bucket/<name>.<namespace> could become more confusing as it's a mix of all the allowed values and placeholders. It may be better to just provide the template and list the supported kind values in description.

We can even leave this out of this change as the values are not fixed. But I still think the new format is relatively better than an undefined helmChartSource value.

"where kind must be one of: (%s), if kind is not specified it defaults to GitRepository",
strings.Join(supportedKustomizationSourceKinds, ", "),
)
return "source that contains the Kubernetes manifests in the format '[<kind>/]<name>.<namespace>', if kind is not specified it defaults to GitRepository"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here

Copy link
Contributor

Choose a reason for hiding this comment

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

Same reasoning as for chart source. Something like

--source [<kind>/]<name>.<namespace>               source that contains the Kubernetes manifests, kind must be one of: (OCIRepository, GitRepository, Bucket), if kind is not specified it defaults to GitRepository

seems like an improvement to me.

"where kind must be one of: (%s)",
strings.Join(supportedHelmChartSourceKinds, ", "),
)
return "source that contains the chart in the format '<kind>/<name>'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as for chart source mentioned above, just without namespace.

@h3nryc0ding h3nryc0ding marked this pull request as ready for review November 13, 2024 00:37
@h3nryc0ding h3nryc0ding changed the title Improve flag help descriptions to show allowed values instead of type names chore(cli): improve flag help descriptions to show allowed values instead of type names Nov 13, 2024
@stefanprodan stefanprodan added the area/ux In pursuit of a delightful user experience label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ux In pursuit of a delightful user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change flags with fixed allowed values to show the actual values
3 participants