-
Notifications
You must be signed in to change notification settings - Fork 4
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
[bug-985]: Update flag descriptions and move flags to relevant commands #238
Conversation
@@ -58,7 +58,7 @@ func TestRoleCreateHandler(t *testing.T) { | |||
|
|||
cmd := NewRootCmd() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why did we do a NewRootCmd() here instead of NewRoleCreateCmd()? Then we don't need to add the role, create, into the Args?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure. The other tests have this pattern and I followed it because it doesn't seem bad.
run e2e test |
Description
admin-token
,addr
, andinsecure
flags only on the relevant commands (karavictl generate, storage, role, rolebinding, tenant) to accurately reflect usageadmin-token
andaddr
as required flags for relevant commands so if the user doesn't provide them, the output is clear about itGitHub Issues
List the GitHub issues impacted by this PR:
Checklist:
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration
Existing unit tests updated.