-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: disable kbcli addons in batch #285
feat: disable kbcli addons in batch #285
Conversation
make kbcli and get follwing err msg, pls fix it first.
make test More unit tests for addons should be added to
./bin/kbcli addon list |
I have been trying to add a test for the disable command part. Have tried to debug using multiple cases but all of them throw the same error - Here are the 3 different code changes I executed but got the same error:
I did also try to use cc - @shanshanying |
When addon cmd calls Hope it will help. |
/approve |
resolve conflicts pls. |
pkg/cmd/addon/addon_test.go
Outdated
|
||
"github.com/apecloud/kbcli/pkg/action" | ||
"github.com/apecloud/kbcli/pkg/testing" | ||
"github.com/apecloud/kbcli/pkg/types" |
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.
This import order does not need to be adjusted.
Strange, I did not close the PR. No idea what happened here 🤔 |
Going for a new PR |
Please head over to this PR: #313 |
The PR contains the code to make the kbcli to disable more than one addon at once. I have not tested the code yet since I am not aware of how to setup kbcli locally for testing purposes and have tried reaching out to @shanshanying for the same.
I respect the busy schedule all the maintainers but would be helpful if I can get KT on how should once setup kbcli locally mainly for testing purposes so that I can make the PR for this issue as well: #278
I am aware that this PR still needs many changes hence I am opening this as a draft PR.
Fixes: #237