Switch package manager from yarn to pnpm#130
Conversation
Standardise this theme on pnpm so CI and contributors agree on one lockfile and one resolution algorithm, matching the shared TryGhost theme convention (Casper, Massively, Pico, et al. pin pnpm@11.5.1). The repo previously carried BOTH a yarn.lock and a committed package-lock.json with npm-based scripts, leaving the package manager ambiguous — installs could resolve differently depending on which tool a contributor reached for. - Pin "packageManager": "pnpm@11.5.1" as the single source of truth (corepack resolves the pnpm version from this field; pnpm/action-setup needs no version input). - Swap npm run -> pnpm in the scripts (zip, pretest). - Add a test:ci / pretest:ci pair: pretest:ci runs `pnpm zip` so CI rebuilds assets before gscan rather than testing stale committed assets/built/*, and test:ci runs `gscan --fatal --verbose` so a fatal compatibility issue fails CI. - Delete the redundant yarn.lock and package-lock.json; pnpm-lock.yaml (lockfileVersion 9.0) is now the only lockfile. - README install/dev/build/zip/test commands now use pnpm. Per-repo decision — NO pnpm-workspace.yaml is needed. pnpm 11 gates dependency install/postinstall build scripts, but this rollup-based theme's tree contains none: the only native package is fsevents@2.3.3, whose lifecycle scripts are build/prepublishOnly (not install/postinstall), so pnpm never flags it. (The gulp-based sibling themes pull in fsevents@1.x + dtrace-provider, which DO have install hooks — hence their allowBuilds lists; this repo does not.) A clean `pnpm install` emits no "Ignored build scripts" warning, so an allowBuilds file would be cargo-culting. CI hardening bundled in (matches the shared convention): - Add test.yml running `pnpm test` on PRs. - Gate deploy-theme.yml behind a test job (needs: test) running `pnpm test:ci`, so a broken build or fatal gscan aborts the deploy instead of shipping it. - Pin action digests, set permissions: contents: read, persist-credentials: false, cache the pnpm store keyed on pnpm-lock.yaml, use --frozen-lockfile. Drop the stale `master` branch trigger.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR migrates the project to pnpm: it sets packageManager to pnpm@11.5.1, updates scripts to use pnpm and the local gscan binary, adds gscan to devDependencies, updates README commands, and adds pnpm workspace build approvals. It adds a pull-request test workflow that installs with a frozen lockfile and runs pnpm test, and updates the deploy workflow to run only on main, declare contents: read, run a test job (pnpm test:ci) on ubuntu-latest, and make deploy depend on that test. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 17-20: The test scripts ("test" and "test:ci") call an unpinned
npx gscan which makes runs nondeterministic; add gscan to devDependencies with a
fixed version (e.g., "gscan": "x.y.z"), update package.json to use the local
binary (keep "npx gscan" or better "gscan" in the "test" and "test:ci" scripts
to rely on the installed devDependency), and remove reliance on unpinned npx by
running pnpm install so CI and local runs use the pinned gscan; update
package.json's devDependencies and adjust the "test"/"test:ci" script entries
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ce32968-57ba-4357-ba6e-2e4088229856
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.github/workflows/deploy-theme.yml.github/workflows/test.ymlREADME.mdpackage.json
Pinning gscan@6.2.1 as a devDependency (matching the sibling themes Casper, Massively, Pico, Editorial) makes the theme-compatibility check reproducible: the version is locked in pnpm-lock.yaml and audited by Renovate, rather than npx silently resolving whatever gscan is latest at run time — which can drift between a contributor's machine and CI, or break a build when a new gscan release changes its ruleset. The `test`/`test:ci` scripts now call `gscan` directly from node_modules/.bin. Adding gscan pulls bunyan -> dtrace-provider@0.8.8 into the tree, which has a native install script. pnpm 11 gates it, so add pnpm-workspace.yaml classifying dtrace-provider as allowBuilds: false — the dtrace bindings are optional and not needed to build or test the theme (same decision as the sibling repos).
Why
Standardise this theme on pnpm so CI and contributors agree on one lockfile and one resolution algorithm, matching the shared TryGhost theme convention (Casper, Massively, Pico, et al. pin
pnpm@11.5.1).The repo previously carried both a
yarn.lockand a committedpackage-lock.jsonwith npm-based scripts, leaving the package manager ambiguous — installs could resolve differently depending on which tool a contributor reached for.Changes
"packageManager": "pnpm@11.5.1"as the single source of truth (corepack resolves the pnpm version from this field;pnpm/action-setupneeds noversion:input).npm run→pnpmin scripts (zip,pretest).test:ci/pretest:cipair:pretest:cirunspnpm zipso CI rebuilds assets before gscan rather than testing stale committedassets/built/*, andtest:cirunsgscan --fatal --verboseso a fatal compatibility issue fails CI.yarn.lockandpackage-lock.json;pnpm-lock.yaml(lockfileVersion 9.0) is now the only lockfile.install/dev/build/zip/testcommands now use pnpm.Per-repo decision — no
pnpm-workspace.yamlpnpm 11 gates dependency
install/postinstallbuild scripts, but this rollup-based theme's tree contains none: the only native package isfsevents@2.3.3, whose lifecycle scripts arebuild/prepublishOnly(notinstall/postinstall), so pnpm never flags it. (The gulp-based sibling themes pull infsevents@1.x+dtrace-provider, which do have install hooks — hence theirallowBuildslists; this repo does not.) A cleanpnpm installemits no "Ignored build scripts" warning, so anallowBuildsfile would be cargo-culting.CI hardening (matches the shared convention)
test.ymlrunningpnpm teston PRs.deploy-theme.ymlbehind a test job (needs: test) runningpnpm test:ci, so a broken build or fatal gscan aborts the deploy instead of shipping it.permissions: contents: read,persist-credentials: false, cache the pnpm store keyed onpnpm-lock.yaml, use--frozen-lockfile. Drop the stalemasterbranch trigger.Verification
pnpm install --frozen-lockfile— cleanpnpm test:ci— no fatal Ghost 6.x compatibility issues, exit 0pnpm zip— producesghost-starter-theme.zip($npm_package_nameresolves under pnpm)