-
Notifications
You must be signed in to change notification settings - Fork 3
ENG-3951: Switch to pnpm and mise for dependency management #1553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying dev-site with
|
| Latest commit: |
24de34b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4e2273e9.dev-site-4ua.pages.dev |
| Branch Preview URL: | https://greg-eng-3951-dev-site-lint.dev-site-4ua.pages.dev |
Summary of ChangesHello @oschwald, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly modernizes the project's development environment by overhauling its dependency and tool management. The core change involves transitioning from npm to pnpm for package management and adopting mise as a centralized tool for version control of various development dependencies. These updates aim to enhance build performance, ensure consistent tool versions across different development and deployment stages, and simplify the project's CI/CD pipeline by removing unnecessary configurations. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is a great pull request that modernizes the project's dependency and tool management by migrating to pnpm and mise. This will significantly improve the developer experience by ensuring consistent tooling and faster dependency installation. The changes are well-structured, and the README.md update is clear and helpful for new contributors. My main feedback is to ensure the migration from npm is complete. I've noticed that some scripts in package.json still use npm and npx. I've left a specific comment on how to address this. Once that's fixed, this PR will be in excellent shape.
| "engines": { | ||
| "node": ">=22.21.1", | ||
| "npm": ">=9" | ||
| "pnpm": ">=10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you've correctly updated the engines to require pnpm, several scripts in this file still use npm and npx. To complete the migration and ensure all commands use the intended package manager, these scripts should be updated.
The following scripts need to be changed:
fix:scripts: should usepnpm runinstead ofnpm run.fix:styles: should usepnpm runinstead ofnpm run.lint:scripts: should usepnpm execinstead ofnpx.format:rawhtml: should usepnpm execinstead ofnpx.
Updating these will make the project's tooling fully consistent and avoid potential issues where the wrong package manager or package version is used.
Introduce mise as the unified version manager for the project, replacing .nvmrc and the manual install-precious script. This ensures consistent tool versions across local development and CI. Tools configured: - node: 22.21.1 (matches existing .nvmrc) - hugo: 0.153.1 (matches wrangler.toml for Cloudflare Pages) - pnpm: 10.28.1 (via aqua backend) - precious: 0.10.1 (via GitHub releases) - dart-sass: 1.97.3 (via GitHub releases, replaces snap install) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
348ec71 to
becda17
Compare
Migrate the package manager from npm to pnpm for faster installs and better disk efficiency through content-addressable storage. Changes: - package.json: Update engines field (npm -> pnpm) - .npmrc: Replace legacy-peer-deps with auto-install-peers - .precious.toml: Replace npm/npx with pnpm/pnpm exec - build.sh: Replace npm run with pnpm run - .gitignore: Add .pnpm-store/ Files removed (replaced by mise): - .nvmrc: Node version now in mise.toml - bin/install-precious: Precious now installed via mise - package-lock.json: Replaced by pnpm-lock.yaml Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace individual tool setup actions with jdx/mise-action for unified tool management. This ensures CI uses the exact same tool versions as local development. Changes: - Remove peaceiris/actions-hugo (Hugo now from mise) - Remove actions/setup-node (Node now from mise) - Remove snap install dart-sass (Dart Sass now from mise) - Add jdx/mise-action@v2 for all tool installation - Add version consistency check between mise and wrangler.toml - Replace npm ci with pnpm install --frozen-lockfile The version check fails CI if mise.toml Hugo version differs from wrangler.toml, ensuring Cloudflare Pages builds use the same version. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace individual tool setup with jdx/mise-action for unified tool management. Precious is now installed via mise instead of the custom install script. Changes: - Remove actions/setup-node (Node now from mise) - Remove ./bin/install-precious step (Precious now from mise) - Remove jq package-lock.json validation (lockfile is now pnpm format) - Add jdx/mise-action@v2 for all tool installation - Add pnpm lockfile validation via --frozen-lockfile --dry-run - Replace npm ci with pnpm install --frozen-lockfile Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The Precious workflow already runs prettier checks via .precious.toml: - prettier-scripts: covers assets/js/** and bin/**/*.ts - prettier-markdown: covers assets/**/*.md and content/**/*.md This workflow duplicates the prettier-scripts check on assets/js/**, so it can be safely removed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace manual tool installation instructions with mise-based setup. All tools (Node.js, Hugo, pnpm, precious, Dart Sass) are now managed through mise.toml for consistent versions across all environments. Changes: - Replace NVM/Homebrew/apt instructions with mise install - Update npm references to pnpm - Simplify installation to three commands: trust, install, pnpm install Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
becda17 to
243fd33
Compare
NTT DOCOMO is a Japanese mobile carrier mentioned in the GeoIP release notes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| "github:houseabsolute/precious" = "0.10.1" | ||
| "github:sass/dart-sass" = "1.97.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
|
Blog links were fixed in: #1545 |
Summary
Changes
Tool Management (mise.toml)
Package Manager Migration
.precious.tomlcommands (npm/npx → pnpm/pnpm exec)build.sh(npm run → pnpm run).nvmrcandbin/install-precious(replaced by mise)CI Workflows
jdx/mise-action@v2for consistent toolingTest plan
mise trust && mise install && pnpm installsucceedspnpm run lintruns (cspell issue unrelated to this PR)./build.shcompletes successfully🤖 Generated with Claude Code