-
Notifications
You must be signed in to change notification settings - Fork 461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
packaging: list files to be published to npm #889
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @nodejs/addon-api any concerns?
TBH, I like the old way better. Npm packages shouldn't target minimum. |
We discussed in the NODE-API meeting and we agreed to give this a try and then see if we get complaints/concerns about the docs not being there. They will still be available through a clone of the github repo. |
PR-URL: #889 Reviewed-By: Michael Dawson <[email protected]>
Landed as 458d895 |
PR-URL: nodejs/node-addon-api#889 Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs/node-addon-api#889 Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs/node-addon-api#889 Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs/node-addon-api#889 Reviewed-By: Michael Dawson <[email protected]>
Hello, rather than exclude files to be published to
npm
via.npmignore
, this PR switches to an inclusive approach viafiles
.As it stands, this change will no longer include the markdown docs in the npm-published tarball as generally people view these on the web rather than via a npm download. I'd be happy to add the docs back to the tarball if required (it's a one line addition).
If we're willing to go without the docs, the resultant tarball is around half its previous size. Given this module is currently downloaded from npm almost 6 times every second (~3.5m/week), this improvement will reduce bandwidth and therefore energy consumption.
Currently published to npm:
What will be published with this change: