awslabs / aws-crt-nodejs

NodeJS bindings for the AWS Common Runtime.
Apache License 2.0
40 stars 27 forks source link

Move tar outside of dependencies #338

Closed TwistedTwigleg closed 2 years ago

TwistedTwigleg commented 2 years ago

Description of changes:

Mirror of https://github.com/awslabs/aws-crt-nodejs/pull/335 but merged with latest changes in main and the package lock regenerated with npm.

Moves cmake-js and tar outside of dependencies and modifies the build script to download tar and cmake-js, use it, and store it in a sub-folder for reuse that is completely separate from the main node modules. This prevents cmake-js and tar from sticking around when it is only needed for building the source and not in the functionality of the CRT itself (and in the case of tar, it is needed only for a single download once to get the source).

With this change, cmake-js and tar is not sticking around in the node dependencies forever when it is only needed for build-time operations.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

TwistedTwigleg commented 2 years ago

Wait on merging even if CI is successful for now - need to double check things.

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 893f8cf41c75eea0482f2fb0366c8514ca4df3ad into 8ae05e0ee7d972bec7cd2c1c69ecea6433f08bbb - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 6 alerts when merging 84a9b6df6b247b2df44ee4f7dc26b944235acab9 into 9300a6699072090dccce731b45bcc5556d52e8ec - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging 0d2a3b081839c3aded6fad97df05ec87f7012569 into 9300a6699072090dccce731b45bcc5556d52e8ec - view on LGTM.com

new alerts:

TwistedTwigleg commented 2 years ago

Thanks for the reviews everyone! Merging into main...