Update all delete commands to support multiple arguments #577
Conversation
tekton-robot
added
the
size/XL
|
The following is the coverage report on pkg/.
|
|
/test pull-tekton-cli-unit-tests |
pkg/cmd/clustertask/delete.go
Outdated
| continue | ||
| } | ||
|
|
||
| fmt.Fprintf(s.Out, "ClusterTask deleted: %s\n", tName) |
There was a problem hiding this comment.
Would it be better to print this just once using the entire list of resources instead of a new line each time? Something like:
tkn clustertask delete foo bar -n ns
results in
ClusterTasks deleted: [foo bar]
There was a problem hiding this comment.
I think the only counterpoint is that by printing them one after the other a user receives a continuous stream of status updates as the deletes occur:
ClusterTask X deleted
Error deleting clustertask Y: the foo bar bazzed
ClusterTask Z deleted
ClusterTask N deletedIn the alternative scenario the user sees the error message but has to wait until the end to see the complete list of successful deletions:
Error deleting clustertask Y: the foo bar bazzed
# some time later ...
ClusterTasks deleted: "X", "Z", "N"I'm ambivalent - which do you prefer?
There was a problem hiding this comment.
In the alternative scenario the user sees the error message but has to wait until the end to see the complete list of successful deletions:
Error deleting clustertask Y: the foo bar bazzed # some time later ... ClusterTasks deleted: "X", "Z", "N"
I like this idea here of showing an error as it happens, but then showing successful deletions once all together.
|
The following is the coverage report on pkg/.
|
|
The following is the coverage report on pkg/.
|
|
/test pull-tekton-cli-unit-tests |
|
@danielhelfand thanks for the review; I've updated the code with the success message changes you suggested. |
|
/lgtm |
|
The following is the coverage report on pkg/.
|
This commit allows the user to delete multiple resources of the same kind at once. For example, to delete multiple tasks the user can issue the command `tkn task delete foo bar baz -n mynamespace` and all three tasks (foo, bar, and baz) will be deleted. The user is only asked to confirm the multiple deletions a single time. Each deletion is handled individually, meaning that it's possible for some deletions to fail while the rest go on to succeed. If any deletes do fail then the command will end as an error.
|
The following is the coverage report on pkg/.
|
|
/test pull-tekton-cli-unit-tests |
tekton-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danielhelfand, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ghost
deleted the
ghost
mentioned this pull request
Changes
Fixes #572
This commit allows the user to delete multiple resources of the same kind at once. For example, to delete multiple tasks the user can issue the command
tkn task delete foo bar baz -n mynamespaceand all three tasks, foo, bar, and baz, will be deleted.The user is only asked to confirm the multiple deletions a single time. Each deletion is handled individually, meaning that it's possible for some deletions to fail while the rest go on to succeed. However, if any deletions do fail then the command will end as an error.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make docsandmake manif needed.make checkRelease Notes