feat: use --name flag instead of positional arg#963
Conversation
BREAKING CHANGE: This changes the use of a positional argument in commands which "get" data. This had a poor user experience as if you do not have completions enabled it is unclear that a positional can be used.
craicoverflow
force-pushed
the
flag-use-describe
branch
from
f10ef2f to
6adcd3c
Compare
craicoverflow
changed the title
|
fyi @bhardesty - This will require update to the guides also, which I can start tomorrow. |
.golangci.yaml
Outdated
| - revive | ||
| - exportloopref | ||
| - godox | ||
| # - godox # Disabled as a TODO should not block code from passing |
There was a problem hiding this comment.
Justification for me to add that was that TODO should be transformed to issue at the time of the PR merge and code should not hold any task tracking elements as that is not the best place to track it.
Ok to keep it. Just giving context so we can completely remove it or keep it
There was a problem hiding this comment.
Understood. I think the difficult here is that the TODOs are for code which do not currently exist on the main branch. I will remove the TODOs though as what you say makes sense.
| _ = cmd.RegisterFlagCompletionFunc("name", func(cmd *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { | ||
| return cmdutil.FilterValidTopicNameArgs(f, toComplete) | ||
| }) | ||
| _ = cmd.MarkFlagRequired("name") |
There was a problem hiding this comment.
[Offtopic] It might be good to have our own helper for that that provides i18n support.
I have been manually validating flags that are required due to lack of i18n support for _ = cmd.MarkFlagRequired("name")
There was a problem hiding this comment.
I don't follow. Something like cmd.MarkFlagRequired(opts.localizer.MustLocalize("flag.id")
craicoverflow
force-pushed
the
flag-use-describe
branch
2 times, most recently
from
a458d00 to
bebcfbb
Compare
craicoverflow
force-pushed
the
flag-use-describe
branch
from
bebcfbb to
89861d6
Compare
|
Ready for review. |
|
I actually forgot the |
|
LGTM |
BREAKING CHANGE: This replaces the positional argument in `rhoas kafka use`, `rhoas kafka describe`, `rhoas kafka delete`, `rhoas kafka topic describe`, `rhoas kafka topic update`, `rhoas kafka topic delete` to a `--name` flag.
3 participants
BREAKING CHANGE: This replaces the positional argument in
rhoas kafka use,rhoas kafka describe,rhoas kafka delete,rhoas kafka topic describe,rhoas kafka topic update,rhoas kafka topic deleteto a--nameflag.Closes #959
Verification Steps
rhoas kafka use|describe|deletepassing--name.rhoas kafka topic describe --name "<topic-name>"Type of change
Checklist