11 Replies
Hi, all:
You may have seen a session on your calendars tomorrow appear and disappear: "Required Status Checks and Merging to Dev Site". This is because we were seeing new failures on testsuites introduced to
main, something that should not have been possible and until recently, wasn't. It's since been fixed.
Here are your takeaways:
* Please do not "approve" PRs that have failing required status checks - particularly on the test suites. Part of approval is validating that the checks are ✅ . You may make exceptions for License/ Sec vulns discovered.
* To that end, @leordev, how do you feel about our removing License/Sec from PR checks? We can catch them in main later and part of me and @finn-tbd feel that they're brittle and teaching bad habits that it's OK to ignore all status checks, not just these.Why this happened:
* A few weeks ago I parallelized the CI so that Kotlin, Swift, Apps, Browser, and JS tests ran at the same time. This was principally to fix the Swift testsuite which cannot run on Ubuntu runners, and also sped up the build.
* This had the 2nd-order effect of leaving our required status check on "Deploy" to be "skipped" in case of failing test suites instead of "fail".
* Therefore our required status check was "skipped", and GitHub required checks treat "skipped" as "hey no problem!". From the docs:
To fix this I've explicitly added all the testsuite checks as required:
@leordev awareness: when we add new testsuite runs to the CI pipelines, we must also add them as a required check here in
main branch protection rules.
Props to @finn-tbd for helping me sort through audit logs and @chrisgiglio for confirming how e were able to replicate this issue.
@devRel You should be protected again going into our release weeks. 🤘🏻Unknown User•8mo ago
Message Not Public
Sign In & Join Server To View
Thanks @AceKYD having a look
@AceKYD You can't do this?
Unknown User•8mo ago
Message Not Public
Sign In & Join Server To View
Ah interesting one moment, this isn't about status checks, it's about your access. One sec.
@AceKYD Try now?
Wait one sec
@AceKYD OK now
Unknown User•8mo ago
Message Not Public
Sign In & Join Server To View
TY!
Following up here since a talk w/ Leo earlier - yeah unfortunately we can't default status checks to "required" and selectively make them optional. That'd have been good but it's not in the GitHub UX 😦