-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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 to electron 7.x #83796
Conversation
8b1d30d
to
5a658f0
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.
Changes LGTM but I suggest this timeline:
- consistent green CI builds on all platforms
- green exploration build for selfhosting during this milestone
- full endgame testing on all platforms on exploration build (including dev setup)
- push to insiders next milestone if no blockers
@@ -366,15 +366,13 @@ export class ElectronMainService implements IElectronMainService { | |||
//#region Connectivity | |||
|
|||
async resolveProxy(windowId: number | undefined, url: string): Promise<string | undefined> { |
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.
(maybe I am missing smth, but still..) Is current version of return type still valid after change that is below?
5a658f0
to
a764ec4
Compare
fbad8db
to
d3f8859
Compare
2c46e2a
to
f8d2758
Compare
f8d2758
to
20ab677
Compare
20ab677
to
22c3db0
Compare
What prevents this to be merged? It is required for linux accessibility so I am waiting impatiently... |
@@ -13,7 +13,7 @@ const webviewId = 'myWebview'; | |||
|
|||
const testDocument = join(vscode.workspace.rootPath || '', './bower.json'); | |||
|
|||
suite('Webview tests', () => { | |||
suite.skip('Webview tests', () => { |
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.
Hey looks like web views are broken under electron due to Prevented webview attach
. I'm looking into it but wonder if you have any idea what that may be?
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.
Pushed fix with c8123de and re-enabled the tests
Caused by #83796 Also re-enables the webview tests
Chromium: 78.0.3904.130
Nodejs: 12.8.1
Electron: 7.1.7