fix: add --set and --set-value-file flag support to delete command and unit tests#10054
Conversation
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables the --set flag for the skaffold delete command and introduces a unit test to verify the presence of the --set and --dry-run flags. Feedback suggests enhancing the test to verify flag binding and ensuring isolation by overriding package-level variables. Additionally, it is recommended to enable the --set-value-file flag for the delete command to maintain consistency with other commands that support manifest overrides.
bogdannazarenko
force-pushed
the
fix/delete-set-flag
branch
2 times, most recently
from
0291ecc to
4ffd77a
Compare
…nd unit tests Signed-off-by: Bogdan Nazarenko <bogdan.nazarenko@outlook.com>
bogdannazarenko
force-pushed
the
fix/delete-set-flag
branch
from
4ffd77a to
ef4594e
Compare
bogdannazarenko
changed the title
|
Seems that the unit test require you to run: ./hack/generate-man.sh |
Signed-off-by: Bogdan Nazarenko <bogdan.nazarenko@outlook.com>
|
@Darien-Lin the failed integration tests are not related to these changes. Looks like they are pre-existing failures |
…d unit tests (GoogleContainerTools#10054) * fix: add --set and --set-value-file flags support to delete command and unit tests Signed-off-by: Bogdan Nazarenko <bogdan.nazarenko@outlook.com> * docs: generate man for cli Signed-off-by: Bogdan Nazarenko <bogdan.nazarenko@outlook.com> --------- Signed-off-by: Bogdan Nazarenko <bogdan.nazarenko@outlook.com>
2 participants
Description
This PR adds support for the
--setand--set-value-fileflag to theskaffold deletecommand, allowing users to override templated manifest fields with key-value pairs during deletion operations.Fixes #10053
Changes made:
cmd/skaffold/app/cmd/flags.goline 764 to include "delete" in theDefinedOnlist for the "set" and "set-value-file" flagscmd/skaffold/app/cmd/delete_test.gowith unit tests to verify that the delete command accepts--set,--set-value-fileand--dry-runflags--setand--set-value-fileflag for the delete commandUser facing changes
Users can now use the
--setand--set-value-fileflags withskaffold deleteto override templated manifest fields:Before:
skaffold deletedid not accept--setor--set-value-fileflagsAfter:
skaffold delete --set key=value --set set-value-file=fileis now supportedThis allows for more flexible deletion operations when working with templated manifests.
Testing
TestNewCmdDeleteindelete_test.gothat verifies both--set,--set-value-fileand--dry-runflags are available