Skip to content

Do not set cancel as true in a reset #10228

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 9, 2022
Merged

Do not set cancel as true in a reset #10228

merged 1 commit into from
Feb 9, 2022

Conversation

benmoriceau
Copy link
Contributor

What

Do not set the state as cancel in case of a reset

@benmoriceau benmoriceau temporarily deployed to more-secrets February 9, 2022 18:25 Inactive
@github-actions github-actions bot added area/platform issues related to the platform area/worker Related to worker labels Feb 9, 2022
@benmoriceau benmoriceau force-pushed the bmoric/fix-reset-instead-of-cancel branch from 18e2d3f to d640434 Compare February 9, 2022 18:50
@benmoriceau benmoriceau temporarily deployed to more-secrets February 9, 2022 18:52 Inactive
@benmoriceau benmoriceau force-pushed the bmoric/fix-reset-instead-of-cancel branch from d640434 to e5d76de Compare February 9, 2022 19:01
@benmoriceau benmoriceau temporarily deployed to more-secrets February 9, 2022 19:03 Inactive
@benmoriceau benmoriceau force-pushed the bmoric/fix-reset-instead-of-cancel branch from e5d76de to 07b12b0 Compare February 9, 2022 19:11
@benmoriceau benmoriceau temporarily deployed to more-secrets February 9, 2022 19:13 Inactive
@benmoriceau benmoriceau force-pushed the bmoric/fix-reset-instead-of-cancel branch from 07b12b0 to bc673df Compare February 9, 2022 19:19
@benmoriceau benmoriceau temporarily deployed to more-secrets February 9, 2022 19:21 Inactive
@benmoriceau benmoriceau force-pushed the bmoric/fix-reset-instead-of-cancel branch from bc673df to e2626c0 Compare February 9, 2022 20:19
@benmoriceau benmoriceau temporarily deployed to more-secrets February 9, 2022 20:21 Inactive
lmossman
lmossman approved these changes Feb 9, 2022
View reviewed changes
Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked through this with Benoit over zoom and I think it makes sense to me. He is going to run a few more tests to make sure the reset behavior is working as expected, so if those succeed then this LGTM

@benmoriceau benmoriceau merged commit c61b8c2 into master Feb 9, 2022
@benmoriceau benmoriceau deleted the bmoric/fix-reset-instead-of-cancel branch February 9, 2022 22:06
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Feb 9, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrhizor jrhizor jrhizor approved these changes

@lmossman lmossman lmossman approved these changes

Assignees
No one assigned
Labels
area/platform issues related to the platform area/worker Related to worker
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benmoriceau @jrhizor @lmossman