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

CI: Review tests and checks required before merge #3680

Closed
tjtelan opened this issue Nov 9, 2023 · 0 comments · Fixed by #3695 or #3706
Closed

CI: Review tests and checks required before merge #3680

tjtelan opened this issue Nov 9, 2023 · 0 comments · Fixed by #3695 or #3706

Comments

@tjtelan
Copy link
Contributor

tjtelan commented Nov 9, 2023

This is a new gap introduced from the last minute transition from Bors to using GH merge queues.

Previously with Bors, we only relied on checking the success of a single job in ci.yml - Done. The implicit behavior we relied on was the dependency chain defined through the jobs.<job_id>.needs keyword.

While configuring the branch protection rules for merge queues, I followed this pattern with only checking for Done.

image
Previously from the master branch protections


Two unexpected things happened

  • The merge check did not block merge from this run even though Done step was skipped
  • The merge continued even though the run failed
    • This was probably bc the requirement for non-failing tests was not enabled w/ merge queue

Out of caution, I've turned on every non-merge_group checked job currently in ci.yml as a requirement for merge. This isn't an accessible solution for the devs, since the config is only in the Github Branch protection UI.

A possible solution is to add more jobs to the dependencies of the Done job, so it explicitly includes all our tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant