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

chore: upgrade dependencies to latest #59

Merged
merged 3 commits into from
Apr 6, 2020

Conversation

erickzhao
Copy link
Member

@erickzhao erickzhao commented Mar 31, 2020

Closes #58

Migrates from electron-download to @electron/get. Biggest motivation: GitHub will give a security warning for any new install of this package due to a vulnerability with the last electron-download version's minimist dependency.

Things to consider:

  • The quiet and strictSSL options are gone. rejectUnauthorized seemed like somewhat of an equivalent to the latter. Please correct me if I'm wrong! @cc malept
  • Converted callbacks to async/await syntax.
  • I snuck in a linting fix command there.

Edit: From discussion below, also:

  • Upgraded extract-zip to 2.0.0.
  • Forced node >= 10

@erickzhao erickzhao requested a review from a team as a code owner March 31, 2020 21:22
@malept
Copy link
Member

malept commented Mar 31, 2020

  • The quiet and strictSSL options are gone. rejectUnauthorized seemed like somewhat of an equivalent to the latter. Please correct me if I'm wrong! cc malept

Yep: https://github.com/sindresorhus/got/blob/v10.7.0/documentation/migration-guides.md#renamed-options

@malept
Copy link
Member

malept commented Mar 31, 2020

Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

FYI, migrating to @electron/get 1.x will require folks to use Node 8.

download-chromedriver.js Show resolved Hide resolved
download-chromedriver.js Show resolved Hide resolved
@erickzhao
Copy link
Member Author

Node 8 reached EOL at the end of 2019, so I think it'd be reasonable to expect a minimum of Node 10 from our users.

Thoughts @malept @codebytere?

@erickzhao erickzhao changed the title refactor: migrate to @electron/get chore: upgrade dependencies to latest Apr 1, 2020
Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

I'm fine with requiring Node 10, I just mention it because it would obviously require a major version bump.

package.json Outdated Show resolved Hide resolved
appveyor.yml Show resolved Hide resolved
download-chromedriver.js Outdated Show resolved Hide resolved
download-chromedriver.js Outdated Show resolved Hide resolved
@erickzhao
Copy link
Member Author

Going to resolve your last issue and rebase the commit history.

@erickzhao erickzhao force-pushed the chore/migrate-get branch 2 times, most recently from 429c47c to c10702b Compare April 1, 2020 19:00
@erickzhao
Copy link
Member Author

CircleCI seems to be failing on macOS, will try to resolve.

@erickzhao erickzhao force-pushed the chore/migrate-get branch 4 times, most recently from 4153bc4 to 960895f Compare April 2, 2020 00:40
@erickzhao
Copy link
Member Author

CI now passing. Seems like you can't directly specify a node version in the config, so I upgraded the xcode to the latest one and that seemed to bump the image up to Node 13.

@malept
Copy link
Member

malept commented Apr 2, 2020

Seems like you can't directly specify a node version in the config, so I upgraded the xcode to the latest one and that seemed to bump the image up to Node 13.

This is what electron-rebuild does:

https://github.com/electron/electron-rebuild/blob/986b33fc44aaa20368830dee44f17a3160554e46/.circleci/config.yml#L14-L21

@erickzhao
Copy link
Member Author

That works even better then, thanks! Will amend again. :)

@erickzhao erickzhao force-pushed the chore/migrate-get branch 6 times, most recently from b224c05 to c75f116 Compare April 2, 2020 16:40
@erickzhao erickzhao force-pushed the chore/migrate-get branch 2 times, most recently from 7458ee0 to c927c62 Compare April 2, 2020 16:48
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.

Package electron-download upgrade to @electron/get
4 participants