electron / get

Download Electron release artifacts
https://npm.im/@electron/get
MIT License
338 stars 106 forks source link

chore!: upgrade got to 11.8.5 #225

Closed clavin closed 2 years ago

clavin commented 2 years ago

Bumps the dependency on got to version 11.8.5, fixing a dependabot alert. got also now includes its own type definitions, so the dependency on @types/got could be removed as well.

malept commented 2 years ago

Also for some reason the tests haven't been triggered

clavin commented 2 years ago

Ah, you're correct. The requirement for got v11 is "node": ">=10.19.0", whereas this package is still "node": ">=8.6". Nice catch!

clavin commented 2 years ago

Added the change to the minimum supported node version requirement to this PR as well just to make sure it doesn't get lost, if we choose to merge this.

malept commented 2 years ago

A couple of things:

  1. The PR title should reflect that this is a breaking change: chore!: upgrade got to 11.8.5
  2. I think you need to re-log-into CircleCI so that the tests will run. If I had to guess it's because of the multiple-orgs-that-require-SSO problem.
clavin commented 2 years ago

Looks like a dependency from this got version (compress-brotli) actually requires the min version to be >=12, even though got is itself only marked as >=10.19. Also have to bump the @types/node dependency here.

vince-fugnitto commented 2 years ago

Any update?

malept commented 2 years ago

See https://github.com/electron/get/issues/224#issuecomment-1177920828, merging this change needs to be coordinated properly.

BlackHole1 commented 2 years ago

Based on https://github.com/electron/get/pull/226#issuecomment-1208824651 discussions. I think we should just upgrade instead of waiting for got to release a new patch version.

cc: @malept

malept commented 2 years ago

We're not waiting for got to release a new patch version.

BlackHole1 commented 2 years ago

What are the reasons that are stopping this PR from being merged now? I want to push for this PR to be merged.

The most important reason, as I understand it so far, is the impact of the minimum node version

BlackHole1 commented 2 years ago

If this impact is so important to us that we cannot merge this PR in a short time, is there another way, for example, if we fork got

electron-bot commented 2 years ago

:tada: This PR is included in version 2.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: