Skip to content

chore(ci): split OS-independent checks into their own jobs - #4734

Open
thetaPC wants to merge 1 commit into
mainfrom
ci-checks
Open

thetaPC wants to merge 1 commit into
mainfrom
ci-checks

Conversation

@thetaPC

@thetaPC thetaPC commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Issue URL: N/A

What is the current behavior?

One matrix job runs every check on Windows and macOS. Several of those read files and report a verdict that cannot vary by operating system, so they run twice for the same answer.

From a recent run, Windows / macOS: Spell Check 27s / 13s, Typecheck 4s / 2s, Test 2s / 1s. macOS bills at 10x Linux, Windows at 2x.

There is also no Linux runner anywhere, while Vercel builds on Linux. Both runners have case-insensitive filesystems, so an import with the wrong casing passes CI and fails production.

What is the new behavior?

Four jobs:

  • Check Admonitions (Linux) — the composite action only. It needs no dependencies, so it no longer waits on npm ci, lint, tests and spellcheck before reporting, and it stops annotating every finding twice.
  • Checks (Linux) — Test, Typecheck, Spell Check
  • Cross-platform on {os} (Windows, macOS) — Lint, check-translations, Check Diff
  • Verify — aggregates the three

Also: cache: npm on both installing jobs, NODE_VERSION declared once at the top, and the matrix job renamed from test, which no longer ran any.

Lint stays in the matrix deliberately. Prettier rewrites line endings and Check Diff catches it, so it is the one check that genuinely differs by OS. It is also the most expensive step at 59s / 32s.

Now that the admonition check has its own job, fetch-depth: 0 on its checkout replaces the targeted git fetch it used to do. That workaround existed to avoid pulling history for a job full of unrelated steps, which no longer applies.

Does this introduce a breaking change?

  • Yes
  • No

Other information

This needs a branch protection change the moment it merges, or merges break.

main and major-10.0 currently require Test on macOS-latest and Test on windows-latest. Those job names no longer exist, so both branches will be waiting on checks that never report.

Switch both to requiring Verify alone. It fails if any of the three fail, and it keeps working if the matrix changes later, which is why it exists. translation/jp has no required checks, so nothing to do there.

One ordering note: GitHub's UI only offers names it has seen, so Verify becomes selectable after this runs once. Worth having someone with admin ready before merging.

The Verify pattern is borrowed from ionic-framework's verify-* jobs, which exist for the same reason.

The two Test on ... checks will never report: this PR renames those jobs, and branch protection still requires the old names. Updating it to require Verify instead clears them.

@vercel

vercel Bot commented Sep 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
ionic-docs Ready Ready Preview Sep 14, 2026 10:49pm UTC

Request Review

@thetaPC
thetaPC marked this pull request as ready for review September 14, 2026 22:58
@thetaPC
thetaPC requested a review from a team as a code owner September 14, 2026 22:58
@thetaPC
thetaPC requested a review from ShaneK September 14, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant