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

Upgrade Electron to 9.3.5 #3000

Merged
merged 8 commits into from Mar 15, 2021
Merged

Conversation

yunyu
Copy link
Contributor

@yunyu yunyu commented Jan 14, 2021

Closes #2860, Closes #2835: electron/electron#24702

Partially solves #2964 (it will now run under Rosetta 2): electron/electron#26572

@develohpanda
Copy link
Contributor

Very cool, thanks for the PR! This has been on our roadmap in order to better support Big Sur.

Windows/Ubuntu CI failed due to bootstrapping, so I've re-run it in case there was an issue with NPM.

@yunyu
Copy link
Contributor Author

yunyu commented Jan 14, 2021

@develohpanda Unfortunately, I don't have access to a windows machine or graphical linux box at the moment. Would you be happen to be able to fix the PR?

For windows, it looks like python 3 is running on the CI but the scripts included use python 2.
For linux, it looks like the curl-config command is missing.

@yunyu yunyu force-pushed the ylin/electron-upgrade branch 2 times, most recently from d3465dd to 7816514 Compare January 14, 2021 04:20
@yunyu yunyu changed the title Upgrade Electron to 9.4.1 Upgrade Electron to 9.3.5 Jan 14, 2021
@develohpanda
Copy link
Contributor

No stress, I was expecting issues with node-libcurl while upgrading, I already had to pin the version during the last release 😂 will take a deeper look when I get a chance to!

@yunyu
Copy link
Contributor Author

yunyu commented Jan 14, 2021

@develohpanda The linux build passes when downgrading Electron to 9.3.5 and upgrading node-libcurl (as there are precompiled binaries). The windows build should pass, except it looks like the github archive server is returning a 503 (it may pass later).

Luckily, this is still a recent enough release to support running on Rosetta 2.

@yunyu
Copy link
Contributor Author

yunyu commented Jan 14, 2021

@develohpanda looks like it went through this time!

@develohpanda
Copy link
Contributor

Awesome! 🌞 I think this will require a degree of QA, but I'll raise it with the team and see where we can fit it in, so I won't merge it immediately but we will get to it! 🤗

@koldoon
Copy link

koldoon commented Mar 4, 2021

This will bring significant improvement for the product, can someone resolve conflicts?

@dimitropoulos
Copy link
Contributor

while I can't say for sure, I think after yesterday's release this particular change will start to bubble up in priority fairly quickly. it's definitely on the radar! I'd love it! haha

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Changes look good!

I'm seeing the following error when running locally for development, having cleared all node modules and bootstrapped again. Very strange, but it's a good sign that the smoke tests have passed (which make network requests as well)! This issue might be isolated to my dev environment, will investigate further.

image

@develohpanda develohpanda requested review from DMarby and dimitropoulos and removed request for DMarby March 5, 2021 03:58
@DMarby
Copy link
Contributor

DMarby commented Mar 5, 2021

@develohpanda I'm getting the same error, it seems like the newer precompiled node-libcurl build that is pulled in alongside the newer electron doesn't correctly statically link to zstd, so we might need to get that resolved upstream before we can proceed with this electron upgrade: JCMais/node-libcurl#274.

image

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Looks to be running now after node-libcurl upgrade 👍🏽

Copy link
Contributor

@DMarby DMarby left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@dimitropoulos dimitropoulos left a comment

Choose a reason for hiding this comment

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

functionally tested on (k)ubuntu: looks good!!!11!1!

@develohpanda develohpanda merged commit 04d0485 into Kong:develop Mar 15, 2021
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.

[Bug] Interface is slow on MacOS Big Sur Broken rounded corners in Big Sur
6 participants