-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
ci: cancel in-progress runs when new changes are pushed up #2997
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2997 +/- ##
===========================================
+ Coverage 82.45% 95.87% +13.42%
===========================================
Files 86 78 -8
Lines 3813 3275 -538
Branches 1284 1150 -134
===========================================
- Hits 3144 3140 -4
+ Misses 669 135 -534 ☔ View full report in Codecov by Sentry. |
On branch pushes, it's critical that a new push not cancel an existing run. I'm fine with this change if it can be restricted to PR runs. |
hmm I'm not sure if that's possible, at least with Could you expand on why its critical that branch pushes are not cancelled by pushes to the same branch? is it because of the release process, or a way of working, or something different entirely? |
Imagine i land two pushes to master/main and the second fails. If the first one is cancelled, how can i quickly know which one introduced the failure? |
You'd look at the CI for the pull requests you opened to land those changes onto the main branch. Don't get me wrong I know that's a bit facetious but to me a situation that requires you to push two changes to your default branch which a lot of people would generally like to think is treated as special and ideally always stable in quick succession when you're not sure that either of them will pass your test suite seems like a problem in of its own. Now compare that to the general cost of running what will (hopefully soon) be 300 CI jobs, some of which are known to be flakey - not only do you have to wait for those to finish, GitHubs concurrency limit per repository is in the 20-60 range, and a small but not zero carbon cost... so to me there's a far bigger advantage in having those jobs automatically cancel when new changes get pushed up. I'll still have a play to see if there's a way we might be able to not have this apply in some conditions but I'm not hopeful it'll be possible to have the cake and eat it too in this case (at least not with native concurrency) |
pfft ok all that writing for nothing - you can make
|
GitHub doesn’t care about its cost, so i shouldn’t either :-) certainly the waiting time can be annoying. On single-maintainer projects, many changes aren’t landed by pull request. |
I’d like it to only apply to PRs, and to not apply to any pushes to any branch, to be clear. |
@ljharb should be good to go now |
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.
LGTM
17223d4
to
f77ceb6
Compare
* fix: backports import-js#3033 * ci: cancel-in-progress when pr is updated backports import-js#2997 * fix: backports import-js#2952 * docs: improve `order` rule docs Backports: - import-js#2640 - import-js#3036
I personally think this is a good idea especially given this projects CI involves over 250+ jobs but I can understand if someone has an alternative workflow that makes it undesirable so feel free to close if you're not a fan 🙂
Related docs: https://docs.github.com/en/actions/using-jobs/using-concurrency