Skip to content

Conversation

@daniel-graham-amplitude
Copy link
Collaborator

@daniel-graham-amplitude daniel-graham-amplitude commented Dec 29, 2025

Summary

(see title)

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: No

Note

Standardizes tooling to pnpm

  • Replace yarn with pnpm across README.md, CONTRIBUTING.md, AGENTS.md, test-server/README.md, and test-server/proxy-test.html
  • Update Bug_report.md environment section to include pnpm and correct casing
  • Switch package scripts to pnpm in package.json for plugin-autocapture-browser (watch-bundle) and align version-file scripts in plugin-session-replay-browser, segment-session-replay-plugin, session-replay-browser, and unified

No runtime code changes

Written by Cursor Bugbot for commit 32b6f23. This will update automatically on new commits. Configure here.

@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Dec 29, 2025

Replace Yarn with PNPM across docs and scripts to migrate amplitude-typescript per AMP-145286

Update documentation and package scripts to use pnpm commands; remove PNPM setup step from publish-single-package.yml.

🖇️ Linked Issues

This PR implements the migration in AMP-145286 under the AMP-145285 epic and contributes to the AMP-91393 initiative by standardizing pnpm usage.

📍Where to Start

Start with the workflow change in publish-single-package.yml to verify CI behavior, then review documentation updates in README.md.

Changes since #1449 opened

  • Added pnpm/action-setup@v4 setup step to the publish-single-package GitHub Actions workflow [32b6f23]

Macroscope summarized 8b992d0.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates all documentation references from yarn to pnpm to reflect the project's migration to pnpm as the package manager.

  • Updates command examples in documentation files (README, CONTRIBUTING, AGENTS)
  • Updates inline comments in package.json scripts to reflect pnpm usage
  • Updates HTML test page instructions
  • Updates issue template to mention pnpm as an installation method

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test-server/proxy-test.html Updates proxy server command from yarn to pnpm
test-server/README.md Updates all development and production commands from yarn to pnpm
packages/unified/package.json Updates autogenerated script comment from yarn to pnpm
packages/session-replay-browser/package.json Updates autogenerated script comment from yarn to pnpm
packages/segment-session-replay-plugin/package.json Updates autogenerated script comment from yarn to pnpm
packages/plugin-session-replay-browser/package.json Updates autogenerated script comment from yarn to pnpm
packages/plugin-autocapture-browser/package.json Updates watch-bundle script to use pnpm
README.md Updates getting started and troubleshooting commands from yarn to pnpm
CONTRIBUTING.md Updates build/test commands and build system reference from yarn to pnpm
AGENTS.md Updates all testing and linting commands from yarn to pnpm
.github/ISSUE_TEMPLATE/Bug_report.md Adds pnpm to installation methods list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Environment
- JS SDK Version: <!--- E.g. 7.1.0 -->
- Installation Method: <!-- I.e. NPM/Yarn or <script> import -->
- Installation Method: <!-- I.e. NPM/yarn/pnpm or <script> import -->
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The capitalization is inconsistent here. The comment uses lowercase "yarn" but uppercase "NPM". For consistency, consider either capitalizing all package managers (NPM/Yarn/pnpm) or using all lowercase (npm/yarn/pnpm). The common convention is to use lowercase for package manager names when referring to the CLI tools.

Suggested change
- Installation Method: <!-- I.e. NPM/yarn/pnpm or <script> import -->
- Installation Method: <!-- I.e. npm/yarn/pnpm or <script> import -->

Copilot uses AI. Check for mistakes.
@daniel-graham-amplitude daniel-graham-amplitude force-pushed the AMP-145286-pnpm-documentation-examples branch from 5052589 to dbd701a Compare December 29, 2025 23:33
@daniel-graham-amplitude daniel-graham-amplitude force-pushed the AMP-145286-migrate-publish-scripts branch from 9be93db to 0cc1a15 Compare December 29, 2025 23:34
@daniel-graham-amplitude daniel-graham-amplitude force-pushed the AMP-145286-pnpm-documentation-examples branch from dbd701a to 4355f5f Compare December 29, 2025 23:34
Copy link
Contributor

@Mercy811 Mercy811 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Base automatically changed from AMP-145286-migrate-publish-scripts to pnpm-migration December 30, 2025 23:21
…cript into AMP-145286-pnpm-documentation-examples
- name: Setup PNPM
uses: pnpm/action-setup@v4

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed pnpm setup action breaks CI workflow

The pnpm/action-setup@v4 action step was removed, but the workflow still uses pnpm commands (pnpm install, pnpm build, pnpm test, etc.). While actions/setup-node@v4 with cache: "pnpm" handles caching, it does not install pnpm itself. All other workflow files in this repository (ci.yml, ci-nx.yml, docs.yml, e2e.yml) correctly include the pnpm setup step before setup-node. Without this action, the pnpm binary won't be available and the workflow will fail.

Fix in Cursor Fix in Web

@daniel-graham-amplitude daniel-graham-amplitude merged commit 253e268 into pnpm-migration Dec 30, 2025
15 of 16 checks passed
@daniel-graham-amplitude daniel-graham-amplitude deleted the AMP-145286-pnpm-documentation-examples branch December 30, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants