Skip to content
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

[meta] auto-merge triggering too early due to skipped pull_request check #819

Closed
shiftinv opened this issue Oct 22, 2022 · 2 comments · Fixed by #851
Closed

[meta] auto-merge triggering too early due to skipped pull_request check #819

shiftinv opened this issue Oct 22, 2022 · 2 comments · Fixed by #851
Labels
bug Something isn't working p: medium Medium priority t: meta Changes to the project itself (CI, configs, etc.)

Comments

@shiftinv
Copy link
Member

shiftinv commented Oct 22, 2022

Pushing to an internal PR naturally triggers all checks that run on push and pull_request events.
Like the other jobs, the check job gets skipped on the pull_request trigger for internal PRs, since the push event already runs the same stuff.

Since the check (push) job depends on other jobs and doesn't run (or even get queued) until they're finished, and the check (pull_request) job gets skipped, there's a window where the requirements set in the branch protection rules are fulfilled (as skipped checks are valid) and the PR gets merged even though some jobs aren't finished yet.

image

For example, this pytest job finished at 13:32:34, while the PR had already been merged at 13:32:16.

@shiftinv shiftinv added bug Something isn't working p: medium Medium priority t: meta Changes to the project itself (CI, configs, etc.) labels Oct 22, 2022
@onerandomusername
Copy link
Member

It looks like if we change some of our workflows and tighten them up a bit as shown here we should solve the issue, since it would cancel the workflow instead of skipping it.

@shiftinv shiftinv changed the title [meta] issues with required CI checks [meta] auto-merge triggering too early due to skipped pull_request check Feb 19, 2023
@shiftinv
Copy link
Member Author

Moved the unresolved part of this issue to a new one, #937.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p: medium Medium priority t: meta Changes to the project itself (CI, configs, etc.)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants