rusackas opened a new pull request, #44738:
URL: https://github.com/apache/superset/pull/44738
### SUMMARY
`pre-commit.yml` ran `npm ci` (~44s) and `yarn install --immutable` (~79s,
no cache) on every single PR, regardless of what actually changed, because the
changed-files detection happened *after* those installs rather than before.
Every hook that actually needs them is already path-gated in
`.pre-commit-config.yaml`:
`oxfmt-frontend`/`oxlint-frontend`/`custom-rules-frontend`/`stylelint-frontend`/`type-checking-frontend`
all require `files: ^superset-frontend/...`, and `oxlint-docs` requires
`^docs/.*\.(js|jsx|ts|tsx)$`. So a PR that never touches `superset-frontend/`
or a JS/TS file under `docs/` was paying for both installs without a single
hook ever using them.
This change:
- Moves the existing "Determine changed files" step to run right after
checkout, before any dependency installs.
- Has it additionally compute `needs_frontend`/`needs_docs` booleans
(mirroring the exact path patterns those hooks already use).
- Gates `Setup Node.js`, `Install Frontend Dependencies`, and `Install Docs
Dependencies` on those outputs.
- Adds yarn caching for the docs install, which had none before (unlike the
npm install, which already caches via `actions/setup-node`).
This job runs on every PR, so it compounds: a backend-only or
docs-prose-only PR no longer pays ~2 minutes of unrelated frontend/docs setup
on every push.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — CI workflow change only, no UI.
### TESTING INSTRUCTIONS
Validated locally:
- `python3 -c "import yaml;
yaml.safe_load(open('.github/workflows/pre-commit.yml'))"` — valid YAML.
- `zizmor --no-exit-codes .github/workflows/pre-commit.yml` — no findings.
- Exercised the new `needs_frontend`/`needs_docs` bash logic standalone
against synthetic file lists (pure-backend, pure-frontend, mixed,
docs-prose-only, docs-JS, empty) — all resolve as expected, including
confirming a `.mdx`-only docs change correctly does *not* trigger the docs
install, while a `.tsx` one does.
This PR's own diff only touches `.github/workflows/pre-commit.yml`, so its
own CI run is a real live test: it should show `Install Frontend Dependencies`
and `Install Docs Dependencies` both skipped.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
