ci(quality): add enable-npm and frontend-path inputs, optional Stylelint leg - #126
Conversation
…int leg Three new inputs for the reusable quality workflow, with matching CONVENTIONS.md documentation: - enable-npm (default true): master switch for the npm ecosystem legs of the security and license jobs, so PHP-only repos without a package.json (e.g. openklant) skip cleanly instead of failing on `npm ci`. Mirror image of enable-php; skipped legs still satisfy required status checks. - frontend-path (default "."): points every npm-side job (Vue Quality, Frontend Checks, npm security/license legs) at a subdirectory holding package.json + package-lock.json, for repos where the frontend is not at the repo root (e.g. woo-website-template-apiv2 with its app in pwa/). Applied via job-level working-directory, npm cache paths, artifact upload paths, and license-checker --start. - enable-stylelint (default true): skip the Stylelint leg for repos without a stylelint npm script, mirroring enable-eslint.
|
Not too much files or a too complicated description: this seems fine |
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (Standard)
Clean, additive, backward-compatible — new inputs default to current behaviour, skipped legs still report so org-wide required checks stay green, and the FRONTEND_PATH env-var pattern keeps caller strings out of interpolation. All 4 CI checks passed.
Three notes to consider — none are blockers, and none map to lines in this PR's diff, so they're here rather than as inline comments.
🟡 SBOM job doesn't honor frontend-path
The SBOM job at .github/workflows/quality.yml:2038-2100 has npm-related steps (Install npm dependencies at L2078, Generate npm SBOM at L2092, and the SBOM merge at L2097) gated by enable-frontend alone. It also doesn't observe enable-npm. For a nested-frontend repo like the woo-website-template-apiv2 example called out in the PR body (enable-frontend: true, frontend-path: pwa, enable-sbom: true), the npm ci at L2078 runs at the repo root and would fail — no package.json there.
Not a regression (this PR doesn't touch the SBOM job), but the "nested frontend" story is incomplete without it. Either extend frontend-path + enable-npm to the SBOM job here, or document the limitation in CONVENTIONS.md and follow up.
🟢 Stale docstring on frontend-setup-command
The existing description at .github/workflows/quality.yml:86 says "The standard root npm ci always runs first". After this PR, that npm ci runs in frontend-path, not always at the repo root. Worth a wording tweak so nested-frontend callers know their frontend-setup-command executes relative to frontend-path (and their examples like cd docusaurus && npm ci resolve from there).
🟢 Two switches for PHP-only repos
CONVENTIONS.md correctly tells PHP-only repos to set BOTH enable-frontend: false AND enable-npm: false. Fine as designed, but a computed default — enable-npm defaulting to enable-frontend — would remove the coordination requirement. Design preference; leave as-is if you'd rather keep the switches orthogonal.
Nice work — the env-var pattern for FRONTEND_PATH, the exit-0-inside-matrix pattern for keeping required checks satisfied, and the CONVENTIONS.md updates make this easy to review.
Summary
Three new inputs for the reusable quality workflow, with matching
CONVENTIONS.mddocumentation, covering repo shapes the workflow couldn't serve yet: PHP-only repos and repos with a nested frontend.Changes
enable-npm(defaulttrue)Master switch for the npm ecosystem legs of the security and license jobs — the mirror image of
enable-php. PHP-only repos without apackage.json(e.g.openklant) set it tofalseso those legs skip cleanly instead of failing hard onnpm ci. Skipped legs still report, so they satisfy the same org-wide required status checks.frontend-path(default".")Points every npm-side job (Vue Quality, Frontend Checks, npm security/license legs) at a subdirectory containing
package.json+package-lock.json, for repos where the frontend is not at the repo root (e.g.woo-website-template-apiv2with its app inpwa/). Applied consistently via:working-directorydefaults,cache-dependency-pathon allsetup-nodesteps,license-checker --startfor the npm license scan.enable-stylelint(defaulttrue)Skips the Stylelint leg of Vue Quality for repos without a
stylelintnpm script, mirroringenable-eslint.Documentation
CONVENTIONS.mddocuments the PHP-only configuration (as the counterpart of the existing JS-only section) and the two new frontend inputs.Compatibility
All three inputs default to the current behaviour — existing callers are unaffected.