-
Notifications
You must be signed in to change notification settings - Fork 221
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
Introduce helm-extra-set-args command line parameter #402
Conversation
431c57f
to
2201116
Compare
@ilmax thanks for the PR, there are some updates in the tests you need to adjust and also can you add some tests for this change as well? |
58d2a73
to
3d30428
Compare
Signed-off-by: ilmax <[email protected]> Signed-off-by: Massimiliano Donini <[email protected]>
- Fixed error when no values are passed in - Added integration test Signed-off-by: Massimiliano Donini <[email protected]>
3d30428
to
feb4b66
Compare
@cpanato I fixed the issue, and added an integration test where I fetch another version of nginx, let me know if you need me to do something more here, Thanks! |
@ilmax thanks for the PR
then commit the changes? |
Signed-off-by: Massimiliano Donini <[email protected]>
@cpanato Sorry, I missed that, done and pushed! |
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.
lgtm
What this PR does / why we need it:
Often times it's required to override the values file when running chart-testing, we have
helm-extra-args
command line option, but that one is used also to uninstall the release so adding set parameters there results in an error in uninstalling the release (see #322, #212, #171)Which issue this PR fixes
fixes #322, fixes #212, fixes #171
Special notes for your reviewer: