Conversation
There was a problem hiding this comment.
Code Review
This pull request updates numerous package.json files across the repository, replacing the 'prepare' script with 'prepublishOnly' to ensure compilation occurs specifically before publishing. The review feedback highlights two opportunities to eliminate redundant steps: first, in 'core/packages/nodejs-proto-files/package.json', where the 'pretest' script performs a duplicate compilation; and second, in 'handwritten/spanner/package.json', where the 'prepublishOnly' script includes an unnecessary manual copy step that is already handled by the 'compile' script.
| "clean": "gts clean", | ||
| "compile": "tsc -p .", | ||
| "pretest": "npm run prepare && npm run compile", | ||
| "pretest": "npm run prepublishOnly && npm run compile", |
There was a problem hiding this comment.
The pretest script runs npm run prepublishOnly followed by npm run compile. Since prepublishOnly already runs npm run compile as its first step, running npm run compile again is redundant and slows down the test execution. Simplifying this to just run prepublishOnly avoids the duplicate compilation.
| "pretest": "npm run prepublishOnly && npm run compile", | |
| "pretest": "npm run prepublishOnly", |
| "clean": "gts clean", | ||
| "compile": "tsc -p . && cp -r protos build && cp -r test/data build/test", | ||
| "prepare": "cp -r protos build && npm run compile", | ||
| "prepublishOnly": "cp -r protos build && npm run compile", |
There was a problem hiding this comment.
The prepublishOnly script explicitly runs cp -r protos build before invoking npm run compile. However, the compile script itself already handles copying the protos directory to build (via tsc -p . && cp -r protos build && cp -r test/data build/test). This makes the manual copy step in prepublishOnly redundant. Simplifying it to just run compile keeps the script clean and avoids duplicate file copying.
| "prepublishOnly": "cp -r protos build && npm run compile", | |
| "prepublishOnly": "npm run compile", |
…s, restore prepare on test fixtures
preparewithprepublishOnlyacross all workspace packages and generator templates sopnpm installdoes not trigger 280+ uncached builds (reducing install time from 6+ min to ~13s).pretestinnodejs-proto-files,logging-utils, andtoolswherepreparewas explicitly invoked.