Repository navigation
Install pre-commit tooling and copier only in the CI jobs that use them - #237
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 32 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| "--install-copier", | ||
| default=False, | ||
| action="store_true", | ||
| help="Install copier. Always installed outside CI.", |
There was a problem hiding this comment.
NIT: help text could be expanded a little to say something of the extent that this flag is only valid in the CI environment so something like that. I first read this as we are installing copier/pre-commit outside this script if not in CI.
There was a problem hiding this comment.
Agreed, that read as if something else installs them outside CI. Both flags, here and in the template's copy, now say the script itself installs them outside CI:
Install copier in CI. Outside CI, this script always installs it.
Changed in 39fb0da and cfd4a02.
Reply drafted by AI (Claude), reviewed and approved by the author before posting.
…alls everything without the CI variable
…#234) ## Link to Issue or Message thread None. Came out of an evaluation of replacing Prettier with Biome across this repo, copier-nuxt-python-intranet-app and an app generated from it. The parts that stood on their own have already merged: #235, #236 (renders `devcontainer.json` without trailing commas, which the Biome check here relies on) and #237 (the install flags this PR's Biome install uses). Tested downstream at both template levels: - child template: LabAutomationAndScreening/copier-nuxt-python-intranet-app#240 - grandchild: a private app generated from copier-nuxt-python-intranet-app, updated from that PR's branch, with CI green including its e2e suites ## Why is this change necessary? Biome already ran in every generated repo through the `biome-check` hook, but `biome.jsonc` disabled its formatter and linter, so all it did was sort imports. Formatting was left to Prettier, whose hook pattern never matched `.js`, `.mjs` or `.cjs` and excluded `.jsonc` and `tsconfig.json`, so those files went unformatted. Biome is also much faster: on a generated app's 170 formatter-relevant files it took 0.09–0.2 s against Prettier's 4.5–6.4 s. Prettier stays for the file types Biome can't format reliably yet. Vue and HTML formatting is still behind Biome's experimental `html.experimentalFullSupportEnabled` flag, and its 2.5.x releases fixed several HTML formatter bugs that changed rendered whitespace. Biome can't format SCSS or Less at all. ## How does this change address the issue? - The Biome config moves from the repo root to `.config/biome.jsonc`, next to the other tool configs. - Biome only discovers `biome.json(c)` in the working directory or its parents, so each runner points at the new location: the `biome-check` hook passes `--config-path` (which also covers CI), and the VS Code extension gets `biome.configurationPath`. - With the config passed explicitly, its globs still resolve from the repo root. - The config turns the formatter on with the `.prettierrc` style. Spaces and a 120-column width are the only settings that differ from Biome's defaults. It also: - enables Tailwind directive parsing so CSS can be formatted again - turns formatting off for `.vue` files - is now the only place that lists what Biome skips: snapshots, `vendor_files`, `generated/` folders, the Strawberry `schema.graphql` export, minified and UMD bundles, and `devcontainer-lock.json` - force-ignores (`!!`) every `.config/biome.jsonc`, itself included, because Biome treats any `biome.jsonc` it is handed as a config and rejects these as a second root config - The Biome hooks become local `language: system` hooks, the setup Biome's docs recommend for running a single version, and run Biome 2.5.15 (up from the v2.1.1 hook) without exclude patterns of their own. A separate `biome-format-config` hook formats the config by running Biome from inside `.config/`, where the file is its own root config. The Prettier hook is narrowed to `\.(vue|html|scss|less)$`. - `install-ci-tooling.py` installs Biome globally through npm as part of the pre-commit tooling from #237, pinned by `biome_version` like uv, pnpm and Task. The hooks and the VS Code extension, which stopped bundling Biome in version 3 and looks in the global npm root, both use that one install. In CI, npm's global bin directory isn't on `PATH` when node comes from `pnpm/setup`, so the installer appends it to `GITHUB_PATH`. - CodeRabbit's Biome is disabled, like its ESLint, ruff and pylint: CI runs Biome through pre-commit, and CodeRabbit would not find the moved config and fall back to its own defaults. - VS Code: the `biomejs.biome` extension formats JSON, JSONC, JS, TS, GraphQL and CSS. Prettier keeps Vue, HTML, SCSS and Less, and its extension is now installed only in repos that use Vue, next to Volar, since no other repo has files it formats. - The reformat touches the `.claude` hook scripts, `.claude/settings/*.jsonc` (trailing commas removed) and the istanbul ESLint rule. The caveman config's `VALID_MODES` stays on one line through a `biome-ignore format` suppression. ## What side effects does this change have? - Generated repos get a one-time reformat on their next copier update. - JS, MJS, CJS, JSONC and `tsconfig.json` are now formatted, and `.jsonc` files lose their trailing commas. - TypeScript changes very little. In a generated app it was six files, all from Biome wrapping Vitest `test(name, options, fn)` calls differently. - Biome doesn't honor `// prettier-ignore` in JS or TS files. Use `// biome-ignore format: <reason>`. The nuxt template's two occurrences are converted in LabAutomationAndScreening/copier-nuxt-python-intranet-app#240. - Child templates with their own `.pre-commit-config.yaml.jinja` (like the nuxt template) need the hook changes ported by hand on their next update, as with any change to that file. Vue templates that use this template's shared pre-commit config, such as copier-vue-package-template and copier-nuxt-static-aws, get the hybrid without extra work. - Generated repos' root `biome.jsonc` moves to `.config/biome.jsonc` on their next copier update. - Running the hooks needs `biome` on `PATH`. Devcontainers get it from the on-create install; anywhere else, run `.devcontainer/install-ci-tooling.py` first, the same way pre-commit, prek and Task get installed. - `pre-commit autoupdate` no longer bumps Biome. Bump `biome_version` in `extensions/context.py` and `BIOME_VERSION` in this repo's `install-ci-tooling.py` instead. - Format-on-save for JS, TS and JSON uses Biome after a devcontainer rebuild installs it and picks up `biome.configurationPath`. Repos without Vue lose the Prettier extension on that rebuild. ## Test plan - Downstream: the nuxt template PR above and a private app generated from it were updated with `copier-update-from-ref` from this branch's head, and CI is green in both, including the app's e2e suites. - `devcontainer.json.jinja-base`: I rendered 52 answer combinations at both template levels (child template roots and generated repos) and ran Biome 2.5.15 on each as `.devcontainer/devcontainer.json`. - None would be reformatted. - The Prettier extension, its config path and the Vue editor settings appear in exactly the 24 renderings for repos that use Vue, and in none of the others. - Exclusions: with Biome 2.5.15, files under `generated/` (including Kiota's `generated/open_api`) and `schema.graphql` are skipped even when passed explicitly, as pre-commit passes them. Other files in the same tree are still formatted. - Config move: - With the config passed via `--config-path`, an exclusion anchored at the repo root (`!src/a.ts`) excluded the file and the `../`-prefixed form didn't, so globs resolve from the repo root, not from `.config/`. - The `biome-format-config` hook restores a deliberately misformatted config. - Installer: run locally with `CI=true`, `--install-pre-commit-tooling` and a scratch `GITHUB_PATH` file, it installed Biome 2.5.15 through npm and appended npm's global bin directory to that file, and `biome` then resolved to the npm install. - Not exercised: - the VS Code extension finding the global npm install in a freshly built devcontainer. Its lookup order (settings, project `node_modules`, global npm root, `PATH`) comes from reading the extension's source. - the Windows branch that takes npm's global bin directory from the prefix itself, since no CI job runs the installer on Windows. ## Other The Biome Vue formatter was checked against a generated app's real files with the experimental flag on. 15 of 38 `.vue` files were reformatted, and all 15 compiled to identical render functions and script ASTs with `@vue/compiler-sfc`. Prettier still handles Vue here because each Biome release can reformat those files again and has fixed render-changing whitespace bugs within the last few versions. Switching Vue over is worth revisiting once that flag is stable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Developer Tooling** * Added Biome as the default formatter for JSON, JavaScript, and TypeScript, with formatting on save in development environments. * Prettier continues to format Vue, HTML, SCSS, and Less files. * Biome is included in development and CI setup, with matching configuration available in generated project templates. * **Style** * Updated formatting settings for consistent indentation and line length. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Link to Issue or Message thread
None. Split out of #234, whose standalone Biome install builds on these flags.
Why is this change necessary?
install-ci-tooling.pyinstalls copier, pre-commit, identify and prek in every CI job, but only the pre-commit job and the lint-matrix job that instantiates the template use them. Every other job, such as unit tests or the e2e suites in generated apps, pays for installs it never uses.How does this change address the issue?
install-ci-tooling.py, both this repo's and the template's, gains--install-pre-commit-tooling(pre-commit, identify and prek) and--install-copier.CIenvironment variable, which GitHub Actions always sets, everything is still installed, so devcontainers and local runs need no flags.install_depsexposes both as inputs, off by default. The pre-commit workflow turns oninstall-pre-commit-tooling, and the lint-matrix job turns on both, in the template and in this repo's own CI.What side effects does this change have?
A CI job that runs pre-commit or copier anywhere else needs the matching
install_depsinput. None does today: in all 8 child templates in the org, and in the templates they ship to generated repos, pre-commit and copier only run in those two jobs.Test plan
uv tool listshows the gating at work: the unit-test job installed only Task, and the pre-commit job, with--install-pre-commit-tooling, installed Task, pre-commit and prek but not copier.CI, it installed copier, pre-commit, identify, prek and Task. WithCI=trueand no flags, it installed only uv, pnpm and Task.Other
🤖 Generated with Claude Code