Protect master branch to prevent accidents
#1073
Comments
|
It just felt unsafe to me that as soon as No problem if we want to turn this off, but defaulting to safety seemed best for now. |
opened
jeffwidman
added a commit
that referenced
this issue
Apr 23, 2024
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
pushed a commit
that referenced
this issue
Apr 25, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I saw this on the home page:
I assume this was an accidental oversight... Or is there a reason this isn't currently enabled?
On other projects I help maintain, typically we setup the following:
masteris locked against pushes by everyone, only merge via PR.The text was updated successfully, but these errors were encountered: