-
Notifications
You must be signed in to change notification settings - Fork 872
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: enhance addon init #4370
Feat: enhance addon init #4370
Conversation
Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com>
Please make sure API is compatible with this change. |
This should work just fine:
We can wait to see the e2e test result to check if it breaks anything. |
Codecov Report
@@ Coverage Diff @@
## master #4370 +/- ##
==========================================
- Coverage 60.62% 58.51% -2.11%
==========================================
Files 343 338 -5
Lines 33772 33775 +3
==========================================
- Hits 20473 19764 -709
- Misses 10617 11416 +799
+ Partials 2682 2595 -87
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
err = cmd.Execute() | ||
assert.ErrorContains(t, err, "required") | ||
|
||
cmd.SetArgs([]string{"test-addon", "--chart", "a", "--helm-repo-url", "https://some.com", "--version", "c"}) |
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.
is this a break change?
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.
Yes, the parameters are adjusted. But this function is for humans, there are no automation tools or other things that uses this, so it won't affect much.
The examples in CLI help have been updated, so the user can refer to help.
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.
well, we're not release it in a formal release, so we can change it for now. But next time, we should keep the consistence
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.
does any docs need to change after this PR?
I don't think there is any documentation that used these parameters. (Only uses The only doc that includes these usages are in CLI help, which has been updated. |
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.
good job! ping @wangyikewxgm
Signed-off-by: Charlie Chiang charlie_c_0129@outlook.com
Description of your changes
--url
to add a ref-objects component (use yaml files from URLs)readme
toREADME
I have:
make reviewable
to ensure this PR is ready for review.backport release-x.y
labels to auto-backport this PR if necessary.How has this code been tested
Unit-tests.
Manually tested with several possible combinations of parameters.
Special notes for your reviewer