Skip to content
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

deps: enable node-gyp iojs.lib download checksum #918

Closed
wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Member

Originally disabled in commit 5de334c ("deps: make node-gyp work again
on windows") due to the then-website lacking the requisite SHASUMS.txt
or SHASUMS256.txt files. The website has a SHASUMS256.txt now so start
checksumming the download again.

R=@piscisaureus, /cc @othiym23

Originally disabled in commit 5de334c ("deps: make node-gyp work again
on windows") due to the then-website lacking the requisite SHASUMS.txt
or SHASUMS256.txt files.  The website has a SHASUMS256.txt now so start
checksumming the download again.
@rvagg
Copy link
Member

rvagg commented Feb 22, 2015

we've had SHASUMS256.txt from the beginning though, not quite sure why this was commented out in the first place

but 👍 from me

@@ -295,8 +295,7 @@ function install (gyp, argv, callback) {
// check content shasums
for (var k in contentShasums) {
log.verbose('validating download checksum for ' + k, '(%s == %s)', contentShasums[k], expectShasums[k])
// TODO(piscisaureus) re-enable checksum verification when the correct files are in place.
if (false || contentShasums[k] !== expectShasums[k]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

false ||

So it wasn't even disabled in the first place?

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently not. It makes sense that everything was fine, given Rod's comment.

bnoordhuis added a commit that referenced this pull request Feb 23, 2015
Originally disabled in commit 5de334c ("deps: make node-gyp work again
on windows") due to the then-website lacking the requisite SHASUMS.txt
or SHASUMS256.txt files.  The website has a SHASUMS256.txt now so start
checksumming the download again.

PR-URL: #918
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@cjihrig
Copy link
Contributor

cjihrig commented Feb 23, 2015

Thanks! Landed in da730c7

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.

4 participants