Skip to content

Conversation

@s100
Copy link
Contributor

@s100 s100 commented Oct 16, 2019

Checklist
  • npm install && npm test passes
  • tests are included
  • commit message follows commit guidelines
Description of change

Fixes #1924. Mitigates #1002.

On Windows, we download node.lib and check its SHA256 sum. That means that on Windows, we also need to download the expected SHA256 sums to check against, always.

@rvagg
Copy link
Member

rvagg commented Oct 16, 2019

yeah, I guess this is reasonable. @richardlau @joaocgreis @bzoz can one of you sanity check this?

Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to be able to specify the lib files using an option as well, but this is certainly better than what is there now. Thanks @s100!

@joaocgreis
Copy link
Member

rvagg pushed a commit that referenced this pull request Oct 21, 2019
PR-URL: #1926
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@rvagg
Copy link
Member

rvagg commented Oct 21, 2019

good stuff, landed in 032db2a

@rvagg rvagg closed this Oct 21, 2019
@rvagg
Copy link
Member

rvagg commented Oct 24, 2019

hey, anyone think this one might be semver-minor?

@joaocgreis
Copy link
Member

I don't see why. Downloads already happen for the .lib files, this adds a new one but fixes the breakage.

rvagg pushed a commit that referenced this pull request Nov 18, 2019
PR-URL: #1926
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fails on Windows when tarPath is overridden manually

4 participants