-
Notifications
You must be signed in to change notification settings - Fork 49
Add --confirm
flag to delete component without asking for confirmation
#568
Conversation
c267bde
to
2b60357
Compare
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.
Looks OK
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.
Just a nit.
2b60357
to
eedd2a9
Compare
Fixes #495 Signed-off-by: knrt10 <kautilya@kinvolk.io>
eedd2a9
to
feb4981
Compare
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.
@knrt10 Thanks for the PR 🎉
LGTM
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
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.
Thanks for the PR!
Looks like I'm too late. I have a few nits. Will leave it up to you to decide what to do about them.
@@ -44,6 +44,7 @@ func init() { | |||
componentCmd.AddCommand(componentDeleteCmd) | |||
pf := componentDeleteCmd.PersistentFlags() | |||
pf.BoolVarP(&deleteNamespace, "delete-namespace", "", false, "Delete namespace with component.") | |||
pf.BoolVarP(&confirm, "confirm", "", false, "Delete component without asking for confirmation.") |
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.
AFAICT we don't end flag descriptions with periods. Example:
lokomotive/cli/cmd/cluster-apply.go
Line 47 in feb4981
pf.BoolVarP(&confirm, "confirm", "", false, "Upgrade cluster without asking for confirmation") |
) { | ||
contextLogger.Info("Components deletion cancelled.") | ||
return | ||
if !confirm { |
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.
Nit - can save some indentation:
diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go
index 76dfee7c..f4393f02 100644
--- a/cli/cmd/component-delete.go
+++ b/cli/cmd/component-delete.go
@@ -80,16 +80,14 @@ func runDelete(cmd *cobra.Command, args []string) {
componentsObjects[i] = compObj
}
- if !confirm {
- if !askForConfirmation(
- fmt.Sprintf(
- "The following components will be deleted:\n\t%s\n\nAre you sure you want to proceed?",
- strings.Join(componentsToDelete, "\n\t"),
- ),
- ) {
- contextLogger.Info("Components deletion cancelled.")
- return
- }
+ if !confirm && !askForConfirmation(
+ fmt.Sprintf(
+ "The following components will be deleted:\n\t%s\n\nAre you sure you want to proceed?",
+ strings.Join(componentsToDelete, "\n\t"),
+ ),
+ ) {
+ contextLogger.Info("Components deletion cancelled.")
+ return
}
kubeconfig, err := getKubeconfig()
Should I revert the changes and create a new PR @johananl? |
I'd create a new one that fixes periods at the end of flag descriptions for the whole project and another that does the indentation optimization. |
on it |
Nah, no need to discard your good work :-) |
fixes #495
Signed-off-by: knrt10 kautilya@kinvolk.io