Don't duplicate CI checks on PRs #1075
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the explanation -- this is quite useful. It is too bad that the GitHub Action Python docs seem to favor |
|
@cclauss good catch! Did you know you can submit a PR to improve the docs? 😊 Every docs page has a "Help us make these docs great!" link at the bottom to open a PR against that exact page. |
|
Yes... https://github.com/github/docs/pulls?q=is%3Apr+author%3Acclauss Will do. |
By default the `pull_request` trigger will run on every push to the PR branch. So we are wasting CI minutes and electricity by running our CI checks twice on every push to a PR branch. Instead, this makes checks run 1x per PR, and then also run on every merge to `master`, to ensure that `master` stays green. This latter check is normally useless, but occasionally if there's drift of some kind between when CI runs on a PR and when it's merged, then this can help identify the issue. A more common pattern is simply to only run on PR's, but given we haven't previously been enforcing "only merge via PR" (#1073) I thought might be best to keep checking `master` as well until that's changed. The one thing we stop doing with this change is checking on push to branches that aren't PR branches... ie, if a maintainer is working on testing something. But they may not even care about running CI on this branch, and if they do, it's easy to run the tests locally, or open a draft PR...So I don't see the point of preserving that behavior.
rbarrois
force-pushed
the
dont-duplicate-CI-checks-on-PRs
branch
from
e14d00a to
7ed1e54
Compare
rbarrois
approved these changes
Apr 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default the
pull_requesttrigger will run on every push to the PR branch.So we are wasting CI minutes and electricity by running our CI checks twice on every push to a PR branch.
Instead, this makes checks run 1x per PR, and then also run on every merge to
master, to ensure thatmasterstays green. This latter check is normally useless, but occasionally if there's drift of some kind between when CI runs on a PR and when it's merged, then this can help identify the issue.A more common pattern is simply to only run on PR's, but given we haven't previously been enforcing "only merge via PR" (#1073) I thought might be best to keep checking
masteras well until that's changed.The one thing we stop doing with this change is checking on push to branches that aren't PR branches... ie, if a maintainer is working on testing something. But they may not even care about running CI on this branch, and if they do, it's easy to run the tests locally, or open a draft PR...So I don't see the point of preserving that behavior.