CI cleanup: cache vendor + node_modules, --no-deps, doc each workflow, rework apispec#419
Merged
turegjorup merged 8 commits intoMay 1, 2026
Conversation
Each PHP workflow currently spends ~30s on a cold composer install for the same composer.lock — across nine workflows that's roughly four minutes of duplicated work per push. actions/cache keyed on composer.lock makes the install a near no-op when the lockfile is unchanged. Dev installs share one key (vendor-php8.4-…); the doctrine workflow's --no-dev install gets its own (vendor-prod-php8.4-…) so prod and dev caches don't clobber each other. composer_install.yaml intentionally stays uncached: its purpose is validating that a fresh prod-mode install works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
phpfpm declares depends_on: mariadb (service_healthy), so every docker compose run on it pulls and boots mariadb plus waits for the healthcheck — wasted work for code-analysis, rector, twig-cs, php-cs-fixer, composer normalize/validate, and the prod-mode composer-install validator. --no-deps starts only the requested service. The compose file is unchanged; doctrine, phpunit, and playwright still pull mariadb because they actually need it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Vitest spends ~31s of its ~34s wall time on a cold npm install; Playwright pays the same toll. actions/cache keyed on package-lock.json turns the install into a near no-op when the lockfile is unchanged. node_modules lives at the repo root and is bind-mounted into the node service via .:/app, so the docker compose setup is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fail-fast only applies to matrix-strategy jobs; declaring it on a single-job workflow is dead config that adds reading overhead and implies a matrix where there is none. Stripping it from the seven workflows that carry it without any matrix:. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eight workflows carried an itk-dev/devops_itkdev-docker "do not edit" header that no longer applies in this repo. Stripping the three-line header preserves the existing ### ### Title / Description / Assumptions docs that follow it. The remaining eleven workflows had no header documentation; they get the same shape (title, one-paragraph purpose, Assumptions list) so a reader can land on any workflow file and learn what it does and what it relies on without leaving the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adapts itk-dev/devops_itksites's two-job api-spec workflow to this repo's v2 spec layout, with one behaviour change: stay quiet when the spec is unchanged. itksites' "no changes detected" comment lands on every PR that touches src/, which is most of them; suppressing that branch removes the noise while keeping the signal — the workflow now only posts on (a) committed spec out of date with the source, (b) non-breaking spec changes (changelog), or (c) breaking spec changes (changelog + failure). --edit-last keeps the PR to one in-place comment across pushes. Pinned oasdiff-action to v0.0.44 instead of @main; path filters restrict the run to PRs that could plausibly affect the spec; the breaking-changes job depends on the export job so a drifting spec isn't compared against the base. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tuj
approved these changes
May 1, 2026
|
|
||
| - name: Check for changes in specifications (json) | ||
| run: git diff --diff-filter=ACMRT --exit-code public/api-spec-v2.json | ||
| - name: Fail if breaking changes detected |
Contributor
There was a problem hiding this comment.
How do we handle if we want to merge breaking changes?
Contributor
Author
There was a problem hiding this comment.
With comment on PR explaining motivation/reasoning.
turegjorup
merged commit May 1, 2026
3eb1659
into
feature/update-infrastructure-for-mono-repo
16 of 17 checks passed
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
2 participants
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.
Description
Stacks on top of #323. Independent cleanups, each in its own commit. No
docker-compose.ymlchanges.vendor/across PHP workflows (actions/cache@v5keyed oncomposer.lock). Wall-time saving is small (Symfony auto-scripts run on every install regardless), but the cache avoids hitting Packagist on every CI run — kept as good citizenship.--no-depsondocker compose runfor code-analysis, rector, twig-cs, php-cs-fixer, composer normalize/validate, and the prod composer-install validator. Compose file unchanged; phpunit/doctrine/playwright keepdepends_on.node_modules/in JS workflows —actions/cache@v5keyed onpackage-lock.jsonforvitest.yamlandplaywright.yaml.strategy.fail-fastfrom seven non-matrix workflows.itk-dev/devops_itkdev-docker. The remaining eleven workflows had no header documentation; they get the same### ### Title / Description / Assumptionsshape.apispec.yaml— adapts itksites' two-job api-spec workflow with one behaviour change: stay quiet when the spec is unchanged. Posts a single edited-in-place comment on (a) drift, (b) non-breaking changes (changelog), or (c) breaking changes (changelog + failure). Pinnedoasdiff-actiontov0.0.44, added apaths:filter, breaking-changes job depends on the export job. Excluded from the benchmark below — its scope changed.Measured timings
Two CI runs back-to-back: the first populated caches; the second is the cache-hot baseline this PR's caching gives going forward.
The wins are driven by
--no-deps(mariadb skip) on database-less workflows, not by vendor caching itself — Symfony's auto-scripts (cache:clear,assets:install) run on everycomposer install, swamping any package-download saving. Vendor cache is kept anyway because every CI run otherwise pulls the same packages from Packagist.node_modules/cache trimsnpm installfrom 32s to 19s on the install step, but ~3s cache save/restore overhead and runner variance net out a modest delta on the wall-time.CI runner variance is ~10-15s for these workflows; single-run benchmarks at this scale are noisy.
Net
Per-PR savings on the slowest workflow (phpunit, the gating one): ~15s. Less than the original estimate, but consistent across runs.
Notes
actions/cache@v5.0.5,oasdiff-action@v0.0.44.feature/update-infrastructure-for-mono-reposo the diff stays scoped.composer install --no-scriptson static-analysis-only workflows would shave ~15s off each by skipping the auto-scripts. Needs a check that PHPStan etc. don't need the container dump.Checklist