Split workflows into ci-pr + ci-main - #50
Conversation
Split verify.yml into ci-pr.yml (PR only) and ci-main.yml (main push only) to eliminate skipped job clutter. Fold publish.yml and notify-main-failure.yml into ci-main with aggregate notifications. Update setup-deps-tarball.sh to reference omoq-ci-main.yml (renamed from omoq-publish-artifacts.yml in moxygen).
|
expected to fail until moxygen pr #69 lands.. neefs to be exercised in o-rly main |
afrind
left a comment
There was a problem hiding this comment.
Overall: Clean companion to moxygen #69, applying the same pattern. The key design decisions from that review
(no workflow_dispatch, cancel-in-progress: false on ci-main, cancel-in-progress: true on ci-pr) are all
correctly carried over.
One Issue Worth Noting
format doesn't gate release
The release job has needs: [build, publish] but not format. So if format fails (unlikely post-PR, but
possible), the release would still publish. The notify job does include format in its needs, so the status is
reported — but the release isn't blocked.
In moxygen there's no format job so this pattern doesn't exist there. Whether to add format to release's
needs depends on how strict you want to be. Given format is a required PR check, broken formatting shouldn't
land on main, but if it ever did, you'd still cut a release.
Correctly Carried Over from Moxygen #69
- cancel-in-progress: false on ci-main ✓
- No workflow_dispatch on ci-main ✓
- Aggregate notify with per-job status symbols ✓
- publish needs: [build] fixing the independent-publish problem ✓
- release needs: [build, publish] ✓
Script Update
The setup-deps-tarball.sh change from omoq-publish-artifacts.yml → omoq-ci-main.yml is correct and is the
direct coupling point with moxygen #69 — these two PRs need to land together (already noted in the PR
description).
Inherited Non-Issues
- JSON injection in notify steps — same pre-existing pattern as moxygen, same low risk
- Branch protection rename from "verify" → "ci pr" — same concern as moxygen, needs updating in repo settings
Nothing blocking here. The format→release gating question is the only real decision to make.
@afrind partially reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on gmarzot).
Summary
verify.ymlintoci-pr.yml(PR only) andci-main.yml(main push only) to eliminate skipped job clutter in the Actions graphpublish.ymlandnotify-main-failure.ymlintoci-main.ymlwith aggregate Slack + email notifications matching the moxygen patternsetup-deps-tarball.shto referenceomoq-ci-main.yml(renamed fromomoq-publish-artifacts.ymlin moxygen PR Fix publish: skip system dep check for --from-release --no-fallback #69)cancel-in-progress: falseon ci-main (mutates external state);trueon ci-pr (safe for PRs)workflow_dispatchon either file (only needed forversion-release)Workflow structure
PR (
ci pr): format, build (linux, asan debug) — no gray circles, no skipped jobsMain push (
ci main): format, build, publish, release, notify — all jobs runCoordination
Must be merged in coordination with moxygen PR #69 which renames the publish workflow file.
Test plan
ci prwith format + build only, no skipped jobsci mainwith full pipelinesetup-deps-tarball.shfinds moxygen artifacts viaomoq-ci-main.ymlThis change is