-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fixes oc delete ignoring --grace-period. #15091
Conversation
[test] |
7a9e62b
to
0d9a533
Compare
@sttts saw you on the upstream |
flake re[test] |
[merge][severity: blocker] here and I'll address the minor comments upstream. |
Evaluated for origin merge up to 0d9a533 |
Evaluated for origin test up to 0d9a533 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3009/) (Base Commit: 83efc33) (PR Branch Commit: 0d9a533) |
@@ -284,7 +284,7 @@ func ReapResult(r *resource.Result, f cmdutil.Factory, out io.Writer, isDefaultD | |||
return nil | |||
} | |||
|
|||
func DeleteResult(r *resource.Result, out io.Writer, ignoreNotFound bool, shortOutput bool, mapper meta.RESTMapper) error { | |||
func DeleteResult(r *resource.Result, out io.Writer, ignoreNotFound bool, gracePeriod int, shortOutput bool, mapper meta.RESTMapper) error { |
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.
as in upstream, a godoc would be nice about this int. time.Duration would be obvious, int is not.
Looks good to me. Only nit: a godoc wouldn't harm for the int. |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1252/) (Base Commit: 349768c) (PR Branch Commit: 0d9a533) (Extended Tests: blocker) (Image: devenv-rhel7_6429) |
Fixes #15060
Picks kubernetes/kubernetes#48582