-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Change "fail" to "stopAndFail" 🧐 #4151
Conversation
This commit proposes changes the wording of the `fail` option for `onError` from `fail` to make it more explicitly clear that this option (which is the default behavior when `onError` is not specified) will both cause the Task to fail AND stop execution of any subsequent steps. This change doesn't propose changing any of the described functionality, just the value of the field. Change to the TEP should be approved before this is merged (tektoncd/community#497) and if we go ahead with it we should do a patch release ASAP to minimize user impact - that being said, this change impacts the default value of the field which users are unlikely to specify explicitly - users are much more likely to start using `onError: continue` and for them there would be no impact.
/hold |
/kind feature |
Since this is technically a backwards incompatible change, we need approval from > 50% of owners. PTAL @tektoncd/core-maintainers |
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.
This definitely sounds clearer!
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.
sounds good, thanks @bobcatfish
/approve ! |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom, dlorenc, imjasonh, jerop, pritidesai, sbwsg 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 |
Thanks all!! /hold cancel |
/lgtm |
Changes
This commit proposes changes the wording of the
fail
option foronError
fromfail
to make it more explicitly clear that this option(which is the default behavior when
onError
is not specified) willboth cause the Task to fail AND stop execution of any subsequent steps.
This change doesn't propose changing any of the described functionality,
just the value of the field.
Change to the TEP should be approved before this is merged
(tektoncd/community#497) and if we go ahead with
it we should do a patch release ASAP to minimize user impact - that
being said, this change impacts the default value of the field which
users are unlikely to specify explicitly - users are much more likely to
start using
onError: continue
and for them there would be no impact.Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes