-
Notifications
You must be signed in to change notification settings - Fork 220
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use stopAndFail
instead of fail
with onError
馃
#497
Conversation
This commit proposes changing 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.
p.s. if anyone can think of better wording than |
thanks @bobcatfish, sounds good 馃憤 The same pattern can be extended to propose debug functionality, /approve |
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.
/approve |
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.
stopAndFail
or failAndStop
? 馃槤
But yeah, nothing against this change 馃懠馃徏
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pritidesai, sbwsg, 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 |
haha good question @vdemeester ! talked it over with some folks and currently leaning toward
But anyway I think either is totally reasonable :D |
Thank you everyone 馃檹 |
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.
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.
This commit proposes changing 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.
PTAL @pritidesai @vdemeester ( @afrittoli I think you're on holiday right now, given this is such a minor change I'm assuming you will be okay with it but strong apologies if not!! 馃檹 )