-
Notifications
You must be signed in to change notification settings - Fork 106
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
fix!: download electron checksum failure #226
Conversation
eb92398
to
c49bf7f
Compare
CI failed reason: |
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.
Can you explain why this change requires got
to be updated? This seems like a pretty large change for a single bug.
It may take time to explain this matter from beginning to end, so please wait. In the meantime, you can start by checking out: electron/fiddle#1156 |
Tips
Why update
|
shasumPath = await downloadArtifact({ | |
isGeneric: true, | |
version: artifactDetails.version, | |
artifactName: 'SHASUMS256.txt', | |
force: artifactDetails.force, | |
downloadOptions: artifactDetails.downloadOptions, | |
cacheRoot: artifactDetails.cacheRoot, | |
downloader: artifactDetails.downloader, | |
mirrorOptions: artifactDetails.mirrorOptions, | |
}); | |
} |
Lines 151 to 153 in 5c81f9a
await sumchecker('sha256', shasumPath, path.dirname(tempDownloadPath), [ | |
path.basename(tempDownloadPath), | |
]); |
Line 60 in 5c81f9a
const downloadStream = got.stream(url, gotOptions); |
This seems like a pretty large change for a single bug.
We talked about "no longer supporting node.js < 10" once before. See: #214 (comment)
Because we have other problems caused by the low version of got #224
I think this is the time to do it.
It would be helpful if you could split out the changes into separate logical commits so that it's easier to review. |
Sorry, I didn't quite understand it. This PR is already a minimal change and cannot be split any further |
You can start with #225 and build upon it. |
OK, I will close this PR as there are duplicate PRs |
Bug context see: electron/fiddle#1156
Break Changes
12.0.0