-
Notifications
You must be signed in to change notification settings - Fork 256
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
Also run tests on Windows Server 2022 GitHub Runner #1176
Conversation
Somehow the CI is still showing a "test" check that is "Waiting for status to be reported" 😐 |
Weird. I wonder if that's because there's a I wonder if a matrix job would be better or be just as confusing for it. |
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
f375fab
to
5c3a8ee
Compare
Drat. The matrix job was pretty easy (and gives a better diff, and more DRY), but it's still showing a mysterious 'test' job as required. For that matter, I'm not sure what actually makes a particular step 'Required' here, but if that's actually a repository configuration rather than in .github/workflows/ci.yml (i.e. the same way DCO and license/cla are hooked up), then that would explain why Edit: Poking around, it's probably a repository setting, the master branch will be set to "Require status checks to pass before merging", and it has the 'test' status check listed there. I suspect that list hasn't been updated as other CI stages have been added either, as I assume it currently lists "lint", "build", test", and "license/cla". Edit again: Yeah, that's it, I can see the protections in the GitHub API.
So trivially changing |
looks like we'd have to remove |
I could also add a If I was making the decision, I'd force-accept this, and then change the GitHub config. We can see that the new test jobs have passed, after all. Although other PRs in-flight would then need to be rebased to pass the new checks, or similarly force-merged. The ideal would be if GitHub could be told " |
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
Low-hanging fruit, but nice to have in place while things like #1160 are in-flight.
Outstanding thoughts: