chore: use lockfiles for NPM dependencies#117
chore: use lockfiles for NPM dependencies#117daniel-graham-amplitude wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces lockfile-based Node tooling to make the repository’s semantic-release execution in GitHub Actions deterministic, replacing ad-hoc npx -p ... installs.
Changes:
- Adds a root
package.jsonto declare the semantic-release toolchain and supporting plugins. - Updates the release workflow to run
npm ciand execute semantic-release vianpm exec. - Ignores
node_modules/in.gitignore.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| package.json | Introduces Node dependencies intended for the release workflow tooling. |
| .gitignore | Adds node_modules/ to avoid committing installed artifacts. |
| .github/workflows/release.yml | Switches semantic-release invocation to lockfile-based install (npm ci) + npm exec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "private": true, | ||
| "dependencies": { | ||
| "lodash": "4.17.21", | ||
| "semantic-release": "17.4.7", | ||
| "@semantic-release/changelog": "5.0.1", | ||
| "@semantic-release/git": "9.0.1", | ||
| "@google/semantic-release-replace-plugin": "1.2.0", | ||
| "@semantic-release/exec": "5.0.0" | ||
| } |
There was a problem hiding this comment.
These packages are only used by the release workflow (not by the Java library at runtime), but they’re declared under dependencies. This makes them appear as runtime deps and can confuse consumers/tools that inspect production dependencies. Consider moving them to devDependencies so they’re clearly build/release tooling only (and npm ci will still install them in CI).
| @@ -0,0 +1,11 @@ | |||
| { | |||
There was a problem hiding this comment.
package-lock.json in this repo has a top-level name field (currently Amplitude-Java), but this package.json doesn’t specify name. That mismatch can cause lockfile churn the next time someone runs npm install/npm ci with a different npm version. Consider adding a name (and optionally packageManager) here to keep the lockfile metadata stable.
| { | |
| { | |
| "name": "Amplitude-Java", |
| - name: Semantic Release --dry-run | ||
| if: ${{ github.event.inputs.dryRun == 'true' }} | ||
| run: | | ||
| npx \ | ||
| -p lodash \ | ||
| -p semantic-release@17 \ | ||
| -p @semantic-release/changelog@5 \ | ||
| -p @semantic-release/git@9 \ | ||
| -p @google/semantic-release-replace-plugin@1.2.0 \ | ||
| -p @semantic-release/exec@5 \ | ||
| semantic-release --dry-run | ||
| npm ci | ||
| npm exec semantic-release -- --dry-run | ||
|
|
||
| - name: Semantic Release | ||
| if: ${{ github.event.inputs.dryRun == 'false' }} | ||
| run: | | ||
| npx \ | ||
| -p lodash \ | ||
| -p semantic-release@17 \ | ||
| -p @semantic-release/changelog@5 \ | ||
| -p @semantic-release/git@9 \ | ||
| -p @google/semantic-release-replace-plugin@1.2.0 \ | ||
| -p @semantic-release/exec@5 \ | ||
| semantic-release | ||
| npm ci | ||
| npm exec semantic-release |
There was a problem hiding this comment.
The workflow now depends on npm ci/npm exec but doesn’t pin or set up a Node.js version. GitHub-hosted runner Node/npm versions can change over time, which undermines the determinism this PR is aiming for and can break releases unexpectedly. Consider adding an actions/setup-node step with an explicit Node version (and optionally npm cache) before running npm ci.
Applies the equivalent of amplitude/Amplitude-Swift#350 in this repository.
Changes
npx -p ...installs to lockfile-based commandsnpm cibefore semantic-release in dry-run and release stepsnpm execpackage.json+package-lock.jsonfor deterministic dependency resolutionnode_modules/in.gitignorewhere needed