ref(build): Rename pack npm scripts to build:npm
#4715
Conversation
size-limit report
|
packages/ember/package.json
Outdated
| "pack": "npm pack", | ||
| "package": "run-s prepack && npm pack && run-s postpack", |
There was a problem hiding this comment.
Looking at this, it probably makes more sense to inline the pre/postpack scripts under package instead of having three scripts here... is there anything I'm missing that would break if I did this?
There was a problem hiding this comment.
Nope, don't think so. Good idea.
Lms24
changed the title
pack npm scripts to packagepack npm scripts to createTarball
There was a problem hiding this comment.
This looks good, and you explained it well!
One small suggestion (and yes, I know this is different than my off-the-cuff suggestion this morning - sorry! Reading it now I'm thinking about it more.): Without context, it might not be clear to all readers that "tarball" === "publishable npm package". Would you be good with yarn build:npm?
Generally yes. It's clear, short and captures what the script is doing. My only concern is that building for me entails some sort of compilation/transpilation going on whereas what This might be just me though, so I'm happy to change it to EDIT: went with |
Lms24
changed the title
pack npm scripts to createTarballpack npm scripts to build:npm
I hear what you're saying, but I think that's focusing too much on implementation details. For me, it all falls under the general heading of "putting stuff together so we can publish it." Looked at in that light, |
rename `pack` npm scripts in all `package.json`s to `package`. Previously, these `pack` scripts could cause confusion in connection with yarn, as `yarn pack` and `yarn run pack` would do different things: While running the former would trigger the pack command of yarn (as in `npm pack`), the latter would execute whatever is defined in the resp. `package.json` undder `pack`. Up to recently, this difference was not noticable in most SDKs because the `pack` npm script would simply be `npm pack`. However given that we're moving everything build-related into its own `build` directory, the pack script must point to the `package.json` inside `build`. As a consequence, `yarn run pack` and `yarn pack` will behave differently which can cause confusion when creating a tarball.
This PR renames the
packnpm scripts in allpackage.jsons tobuild:npm. Previously, thesepackscripts could cause confusion in connection with yarn, asyarn packandyarn run packwould do different things: While running the former would trigger the pack command of yarn (as innpm pack), the latter would execute whatever is defined in the resp.package.jsonunderpack.Up to recently, this difference was not noticeable in most SDKs because the
packnpm script would simply benpm pack. However, given that we're moving everything build-related into its ownbuilddirectory (#4688, #4641), the pack script must point to thepackage.jsoninsidebuild. As a consequence,yarn run packandyarn packwill behave differently which can cause confusion when creating a tarball.With this PR,
yarn (run) build:npmwill create an NPM package (tarball). Besides modifying all SDKpackage.jsons, this PR also adjusts the packing script in the root-levelpackage.jsonand hence in the the packaging command in thebuild.ymlfile.It should be noted that the purpose of this PR is to avoid confusion when creating NPM packages, not to fix anything that would otherwise be "broken". CI also works currently with the
builddirectory change as the artifact creation job does not callyarn packbutyarn pack:changed.The motivation for making this change emerged after a conversation about adjusting
yarn packtoyarn run packto get a correct tarball.ref: https://getsentry.atlassian.net/browse/WEB-688