-
Notifications
You must be signed in to change notification settings - Fork 45
Conversation
/cc @joaomoreno @bpasero |
function download(opts, cb) { | ||
var repo = opts.repo || 'atom/electron'; | ||
var github = new GitHub({ repo: repo, token: opts.token }); |
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.
We still need to support this configuration for the internal builds.
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.
Ah I see that we can specify a mirror, that should do the trick
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.
How to do that?
Thanks for working on this @codebytere |
Cool! |
@deepak1556 Would you like to drive this PR's adoption in VS Code? I've invited you as a collaborator. |
Thanks @joaomoreno , would be happy to do it. |
Another idea @bpasero and I just discussed about: how about just dropping this? Do we still need this module at all? |
I was referring to https://github.com/electron-userland/electron-forge in the standup today. |
What do you all envision the alternate path looking like, should we choose to take it? I'm happy to switch up my approach or try to PR something into vscode itself |
c2bcdf1
to
44ca6ab
Compare
44ca6ab
to
444e748
Compare
@codebytere No, no, this was more of a comment for us, VS Code team. |
👋 hey folks, is there anything i can do to help move this PR forward this week? |
@codebytere last week was testing in vscode during which the master branch will be frozen, I plan to move this forward tomorrow. |
27972f5
to
cc5b4e9
Compare
Thanks @codebytere for the fast turnaround, the following changes are also needed diff --git a/src/download.js b/src/download.js
index f408555..5363fdd 100644
--- a/src/download.js
+++ b/src/download.js
@@ -1,7 +1,8 @@
'use strict';
var path = require('path');
-const { downloadArtifact, getDownloadUrl } = require('@electron/get');
+const { downloadArtifact } = require('@electron/get');
+const { getDownloadUrl } = require('./util')
var semver = require('semver');
var rename = require('gulp-rename');
var es = require('event-stream');
@@ -31,15 +32,20 @@ function download(opts, cb) {
version: opts.version,
platform: opts.platform,
arch,
- token
+ token: opts.token
};
if (opts.repo) {
getDownloadUrl(opts.repo, downloadOptions)
- .then(({err, downloadUrl}) => {
+ .then(({err, downloadUrl, assetName}) => {
if (err) return cb(err)
- downloadOptions['mirrorOptions'] = { mirror: downloadUrl }
+ downloadOptions['mirrorOptions'] = {
+ mirror: downloadUrl,
+ customFilename: assetName
+ };
+ downloadOptions['artifactName'] = assetName;
+ downloadOptions['unsafelyDisableChecksums'] = true;
downloadArtifact(downloadOptions).then(zipFilePath => {
return cb(null, zipFilePath)
diff --git a/src/util.js b/src/util.js
index 54499fa..2ace0fb 100644
--- a/src/util.js
+++ b/src/util.js
@@ -7,8 +7,8 @@ function startsWith (haystack, needle) {
return haystack.substr(0, needle.length) === needle;
};
-async function getDownloadUrl(repo, { version, platform, arch, token }) {
- const [owner, repo] = repo.split('/');
+async function getDownloadUrl(repo_url, { version, platform, arch, token }) {
+ const [owner, repo] = repo_url.split('/');
const octokit = new Octokit({ auth: token });
const { data: release } = await octokit.repos.getReleaseByTag({
@@ -46,7 +46,7 @@ async function getDownloadUrl(repo, { version, platform, arch, token }) {
});
const { url, headers } = requestOptions;
- headers.authorization = `token ${token}`;
+ headers.authorization = `token ${token}`;
const response = await got(url, {
followRedirect: false,
@@ -54,7 +54,7 @@ async function getDownloadUrl(repo, { version, platform, arch, token }) {
headers
});
- return { error: null, location: response.headers.location };
+ return { error: null, location: response.headers.location, assetName: asset.name };
}
module.exports = { |
cc5b4e9
to
e942403
Compare
features from the previous version are preserved now,
There are two issues that still needs addressing
|
e942403
to
97bfded
Compare
99a2415
to
d9a070a
Compare
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 with minor style changes.
9708956
to
e582f5e
Compare
e582f5e
to
f3f50e7
Compare
15302b2
to
1f3bee0
Compare
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.
Thanks for working through my review 😄
Good to go now. @joaomoreno if you can make a new release, will adopt this in VSCode. Thanks!
This PR updates
gulp-atom-electron
to use Electron's recommended@electron/get
for simpler downloading and automatic caching. Some other benefits include the ability to download nightly releases as well as specify custom mirrors.Tested locally with Electron versions
v7.2.1
andv7.2.3
to confirm efficacy of change when runningyarn electron
in vscode itself.cc @deepak1556