Fix --keep Description and Remove Need to use with --all Flag #921
Conversation
|
|
tekton-robot
added
the
size/M
|
The following is the coverage report on pkg/.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
tekton-robot
added
the
approved
|
/hold Slight test issue I would like to correct |
tekton-robot
added
the
do-not-merge/hold
pkg/cmd/taskrun/delete_test.go
Outdated
| dynamic: seeds[4].dynamicClient, | ||
| input: seeds[4].pipelineClient, | ||
| inputStream: nil, | ||
| wantError: false, | ||
| want: "All but 2 TaskRuns deleted in namespace \"ns\"\n", | ||
| }, | ||
| { | ||
| name: "Delete all keeping 1 with --all flag", | ||
| command: []string{"delete", "-f", "--keep", "1", "-n", "ns"}, |
There was a problem hiding this comment.
Should have --all flag to verify this won't be a breaking change.
|
The following is the coverage report on pkg/.
|
|
/hold cancel |
tekton-robot
removed
the
do-not-merge/hold
Slight cleanup of
--keepflag fortkn pr delete/tkn tr delete. With this change, users will no longer need to use--allwith--keepbut still can. This pull request will set--allto true when--keepis greater than 0. This is needed since--keeprelies on funcs used with--all.This pull request also corrects the description to clearly communicate what PipelineRuns/TaskRuns are kept when using
--keep. #800 is incorrect in stating that most recent PipelineRuns/TaskRuns are not kept. I believe the original issue was opened because of #802, which caused confusion cause of what was kept.This pull request does not solve #800 as the behavior should probably be changed later on, but it does clean up certain user experience aspects of
--keep.Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make checkmake generatedRelease Notes