-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
revert: test(client) wrap cmd.SetArgs to fix bugs for cmd.SetArgs (#18876) #18902
Conversation
…osmos#18876)" This reverts commit ae19acc.
WalkthroughThe overall change involves updating a Go test file to refine how command arguments are set for testing. The update replaces an internal utility function with a command method for argument handling and makes adjustments to flag settings. These changes are aimed at improving the test setup for a specific command function without altering any public or exported entities. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
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.
Ok, so we are waiting for changes in cobra instead, if i understand correctly right?
Yep, I will try my best to promote it. If |
This reverts commit ae19acc. #18876
Description
I just found the bug of the implementation of
SetArgs
ininternal/testutil
, it would get unexpected default value when the flag field type isSliceValue
(Append instead of replace). See detail info in comment.If
spf13/cobra
won't support forReset() error
feature, I will choose another way to implement this. But for now, I think it would be better to revert commit first.Sorry about this, I didn't cover a full test for wrapped
SetArgs
.