Update README#464
Conversation
![constructor-claude-bedrock[bot]](https://avatars.githubusercontent.com/u/9828194?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR improves the README by documenting the npm version gotcha caused by ignore-scripts=true in .npmrc, and restructures the existing prepare/husky note for clarity. The changes are accurate, well-explained, and fill a real gap in contributor documentation.
Inline comments: 2 discussions added
Overall Assessment: ✅ Pass
| npm run coverage # run tests and serves coverage reports from localhost:8081 | ||
| npm run docs # output documentation to `./docs` directory | ||
|
|
||
| npm version <patch|minor|major> --ignore-scripts=false # bump version (see note below) |
There was a problem hiding this comment.
Suggestion: The --ignore-scripts=false flag overrides the .npmrc setting globally for that invocation, which is exactly right. However, the inline comment # bump version (see note below) is the only forward-reference to the note in the code block — the note itself now appears immediately after, so the comment still works. One minor readability improvement: consider shortening the comment to just # see note below since "bump version" is already conveyed by the command itself, reducing redundancy.
| > **Note:** `.npmrc` sets `ignore-scripts=true` for supply-chain safety, so lifecycle scripts (including husky's hook installation) do not run automatically on `npm install`. Run `npm run prepare` once after cloning to enable the `pre-push` lint hook. | ||
| > **Note:** `.npmrc` sets `ignore-scripts=true` for supply-chain safety, so lifecycle scripts do not run automatically. This affects two workflows: | ||
| > - **husky git hooks:** run `npm run prepare` once after cloning to enable the `pre-push` lint hook. | ||
| > - **`npm version`:** the `version` lifecycle script (which regenerates `src/version.js`, docs, and the `./dist` bundle and stages them into the release commit) is skipped. Always pass `--ignore-scripts=false` when bumping the version so these artifacts are regenerated. |
There was a problem hiding this comment.
Suggestion: This is accurate and matches what package.json's version script actually does (node ./src/generateVersion.js && npm run docs && git add ./docs/* && npm run bundle && git add -A ./dist && git add ./src/version.js). For extra safety and discoverability, consider adding a note that --ignore-scripts=false only needs to be paired with npm version (not npm publish, which uses prepublish), so contributors aren't confused about whether they need the flag for other npm lifecycle events.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the README to document the impact of .npmrc’s ignore-scripts=true on common workflows, and to ensure version bumps regenerate required artifacts.
Changes:
- Adds an
npm versionexample command that explicitly enables lifecycle scripts. - Expands the existing note to clarify which workflows are affected and what to do.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| npm run coverage # run tests and serves coverage reports from localhost:8081 | ||
| npm run docs # output documentation to `./docs` directory | ||
|
|
||
| npm version <patch|minor|major> --ignore-scripts=false # bump version (see note below) |
No description provided.