build: clarify makefile target names and restructure ci#356
Merged
Conversation
Rename Makefile targets to a consistent area-first `<area>-<action>` convention (core-*, api-*, gen-*, audit-*, fmt-*), add `test-all` as the deep all-C++ gate, and delete redundant/low-value targets (validate-cpp, probe-cpp, check-core-dev, gen-check) together with their sources and the mxtest-validate CMake target. Restructure GitHub Actions CI from ten always-on jobs into a leaner set: changes, test-linux (make test-all), test-macos, test-windows (new, native MSVC), swift, quality (fmt-check + regen drift), and a gen job gated on gen/** changes. Move code coverage out of the always-on run into a new manual-only coverage.yaml (workflow_dispatch or a `/coverage` PR comment), still posting the same gcovr tables to the PR. pr-comment.yaml is unchanged (still posts the gen-quality comment for fork PRs). Docs, skills, and instruction files updated to the new target names.
webern
added
non-breaking
Claude
gen-quality
|
The changes-job filter OR'd 'gen/**' with '!gen/**/*.md', so picomatch evaluated the negation as a standalone matcher that any non-gen-markdown path satisfies -- the gen filter fired on essentially every PR. Add predicate-quantifier 'every' so a changed file must be under gen/ AND not markdown to trip it.
Owner
Author
|
/coverage |
Windows was the CI long-pole at ~19 min (17m42s in the build step alone) because the native macOS/Windows/Swift jobs had no compiler cache -- only the Linux jobs did, via ccache baked into the mx-sdk image. - test-windows: switch from the Visual Studio generator to Ninja so sccache can hook in (the VS generator ignores CMAKE_<LANG>_COMPILER_LAUNCHER), set up MSVC via ilammy/msvc-dev-cmd, and force /Z7 embedded debug info (sccache can't cache /Zi's shared-PDB writes). Single-config Ninja also collapses the binary paths from build/api/Debug/ to build/api/. - test-macos: replace the build/-dir cache (keyed on src/**, so busted by every source change) with ccache via compiler-launcher env vars. - swift: cache the SwiftPM .build tree. - Bump actions/checkout v4 -> v5 and actions/github-script v7 -> v8 (both now on Node 24; Node 20 is deprecated on GitHub Actions runners).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
CI was hard to understand so I am attempting to simplify it.
Summary
The Makefile target names and the CI job layout had both drifted into something
hard to follow. This pass makes both legible around one mental model: a single
product (
mx::api) on a substrate (mx::core), plus a dev tool (gen/).Makefile:
<area>-<action>scheme:core-build/core-roundtrip-test/core-unit/core-coverage,api-lib/api-build/api-test/api-roundtrip/api-coverage,plus
gen-test,audit-test,fmt-check, and theapi-roundtrip-*helpers.test-allruns every C++ suite at once (core roundtrip + unit + api-testvalidate-cpp(plus
mxtest/validate/and themxtest-validateCMake target),probe-cpp(plus
mxtest/probe/),check-core-dev, andgen-check(its rename andcollision checks already run during real generation).
CI, from ten always-on jobs down to a leaner set:
test-linux(make test-all),test-macos(native AppleClang, api tier),test-windows(new — native MSVC, driving CMake directly),swift,quality(fmt-check + generated-output drift), and agenjob gated onchanges under
gen/.test-windowsis a required check and is expected to stay red until the codeis MSVC-clean; it exists so that fallout runs on every PR from now on.
coverage.yaml, triggered by the workflow's Run button or a/coveragecomment on a PR. It posts the same gcovr tables to the PR as before.
pr-comment.yamlis unchanged; it still posts the gen-quality comment for forkPRs. Docs, skills, and instruction files are updated to the new names.
Testing
This is a CI/build change, so the workflows running on this PR are the real
test. Verified locally:
make helprenders, andmake -n test-allresolves the full dependencychain in both the in-container and docker-wrapper branches
target names remain outside
docs/ai/design/(kept as historical)References
smallenum value conflicts with Windows SDKrpcndr.hmacro #354 (thesmall/rpcndr.hMSVC conflict the new Windows job willsurface) and fix: support optional member detection with MSVC #353 (the recent MSVC build fix this CI job guards going forward)
Generated by Claude Code