-
Notifications
You must be signed in to change notification settings - Fork 616
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
Cancel in progress workflows on new commit #6759
Conversation
@@ -16,6 +16,9 @@ on: | |||
- synchronize | |||
# trigger workflow if PR is marked ready for review. | |||
- ready_for_review | |||
concurrency: | |||
group: ${{ github.ref }} |
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.
do we want to group by workflow just in case? https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-concurrency-and-the-default-behavior
I don't think this would have any tangible diff, though.
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.
I think we don't need to right? I think it's fair to say that if we push a new commit we want to cancel everything that was running and just re-run them from scratch with the new code
@@ -16,6 +16,9 @@ on: | |||
- synchronize | |||
# trigger workflow if PR is marked ready for review. | |||
- ready_for_review | |||
concurrency: | |||
group: ${{ github.ref }} | |||
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} |
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.
cancels if not on main, right?
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.
yep this is my understanding of it, we will leave the ones that run on main alone
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.
added this for e2e-fork too, lgtm, would be nice to merge this first thing today!
Description
We can start with this on just the regular e2es and wasm e2es which should be the ones that will have the most redundant runs.
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.