Suggestion: re-enable build-test-coverage CI on master and ready PRs #158
ottobolyos
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
@PatrickRitchie, I wanted to put an idea on your radar. The
build-test-coverageworkflow (.github/workflows/dotnet.yml) was disabled at the workflow level a few minutes after #139 merged—state: disabled_manuallyper the GitHub API, with the timestamp matching the master push event for the merge. As a result, the rewritten workflow that landed with #139 has actually never executed: the workflow has zero recorded runs since the rewrite. Ready-for-review PRs in the campaign queue (#141 today; #145, #143, #147, #146, #144, #142, #148, #140, #149, #150, #155 as they flip ready) all show an empty status-check rollup as a knock-on effect.I don't know whether the disable was a deliberate gate ('I want to verify the new YAML before letting it run automatically') or an interim concern about something specific. Either way, the suggestion is: re-enable the workflow so master pushes and ready PRs get a green-or-red CI signal. The maintenance burden is low—I'd be happy to help.
Current state—already configured for ready-only, no edit needed
It's worth highlighting because it's a small surprise: the rewritten YAML in #139 already excludes draft PRs from CI. The
on:block listspull_request: types: [opened, synchronize, reopened, ready_for_review]and thebuild-and-testjob carriesif: github.event_name == 'push' || github.event.pull_request.draft == false. So if you re-enable as-is:masterpushes (post-merge): trigger CI.ready_for_reviewevent triggers CI automatically; no manual re-run needed.So no YAML edit is required to get the 'ready PRs only' behaviour—just the re-enable. (The previous-generation YAML, which has been on the repo since 2022, gated
pull_requestonbranches: [main]instead ofmaster, so it had effectively been a no-op on the PR side for the entire history of the repo. The rewrite in #139 fixed that branch name plus added the matrix and the draft gate.)Pros
masteritself currently has zero CI signal (the rewrite landed but never ran). Re-enabling means every merged PR gets verified on master immediately afterwards—useful as a 'did the merge introduce something the per-PR run missed' check.Cons / things to weigh
Settings → Actions → General → Fork pull request workflows from outside collaboratorslets you require approval for every outside contributor, not just first-timers.ubuntu-latestandwindows-latest(.NET 8 + 9). If wall-clock time becomes a queue concern—e.g. multiple pushes within a few minutes—trimming the PR matrix (ubuntu-only on PR; full matrix onmasterpush) is a small change. It would be easy to add later if needed; it isn't a blocker for re-enable.How to re-enable
Three steps, all on your side:
Settings → Actions → General—confirm the actions policy allows the workflow'sactions/checkout+actions/setup-dotnet(the workflow already pins them to commit SHAs per GitHub's hardening guidance).Workflow permissionsshould beRead and writeso the workflow can upload TRX + coverage artifacts.Actions tab → build-test-coverage workflow → Enable workflow—single button click; flipsdisabled_manuallytoactive.workflow_dispatchfrom the same Actions UI to verify the new YAML works on master before the next PR push fires it. The workflow doesn't currently include aworkflow_dispatchtrigger—I'd be happy to add one in PR test: comprehensive test-suite overhaul (100% coverage + E2E + compliance harness) #150 if a manual-trigger button would be useful for ad-hoc smoke checks.Where to land follow-up CI changes
If any further workflow tweaks come out of this discussion (matrix trim,
workflow_dispatchbutton, lighter test filter on PR runs, etc.), PR #150 (test/coverage-and-compliance) already touches.github/workflows/dotnet.ymland is the natural landing spot—I'd be happy to fold the changes in there. If you'd rather isolate the CI changes from the test-harness PR, I'd be equally happy to open a small separate PR. Just say which you prefer.If the suggestion lands, the campaign queue gets per-PR CI signal automatically as each PR flips to ready, and master gets ongoing post-merge verification. If you'd rather keep the workflow disabled for now, that's no problem—the campaign PRs can merge on the strength of code review + clean merge state regardless. I just wanted to flag the option.
Beta Was this translation helpful? Give feedback.
All reactions