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

[MM-61271] Upgrade to Electron v33.0.2 #3181

Merged
merged 3 commits into from
Oct 25, 2024
Merged

[MM-61271] Upgrade to Electron v33.0.2 #3181

merged 3 commits into from
Oct 25, 2024

Conversation

devinbinnie
Copy link
Member

@devinbinnie devinbinnie commented Oct 24, 2024

Summary

Upgrade to Electron v33.0.2. Some deprecations were addressed, but otherwise nothing major should be changed.

Ticket Link

https://mattermost.atlassian.net/browse/MM-61271

Upgrade to Electron v33.0.2

@devinbinnie devinbinnie added 2: Dev Review Requires review by a core committer CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 24, 2024
@devinbinnie devinbinnie added this to the v5.10.0 milestone Oct 24, 2024
@devinbinnie devinbinnie requested review from a team, wiggin77 and pvev and removed request for a team October 24, 2024 18:52
@@ -0,0 +1,33 @@
diff --git a/node_modules/electron/electron.d.ts b/node_modules/electron/electron.d.ts
Copy link
Member Author

Choose a reason for hiding this comment

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

This patch is needed due to an issue in Electron's typing for this module that caused a bug. A fix is waiting to be merged, but for now we'll use this patch.
electron/electron#44391

} else {
focusedWindow.webContents.openDevTools({mode: 'detach'});
}
click() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This used to matter when we had the Settings and Main windows, but now we just have the main one so we can assume we're using that one.

@devinbinnie devinbinnie added the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
Copy link

Here are the test results below:

@github-actions github-actions bot removed the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
@devinbinnie devinbinnie added the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
Copy link

Here are the test results below:

@github-actions github-actions bot removed the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
@devinbinnie devinbinnie added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Oct 24, 2024

#if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 11 \
- && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION > 7)
+ && defined(V8_MINOR_VERSION) && (V8_MAJOR_VERSION > 12 || (V8_MAJOR_VERSION == 11 && V8_MINOR_VERSION > 7)))
Copy link
Member Author

Choose a reason for hiding this comment

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

The most recent Electron version bumped V8 to version 13, but that hasn't been updated in nan yet. A PR to do so is already open: nodejs/nan#979

@devinbinnie devinbinnie added the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
Copy link

Here are the test results below:

Test Summary for Linux on commit d16db26

New failed tests found on Linux:

  • menu/view MM-T816 Toggle Full Screen in the Menu Bar

Test Summary for macOS on commit d16db26

New failed tests found on macOS:

  • copylink MM-T125 Copy Link can be used from channel LHS

@github-actions github-actions bot removed the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
Copy link

Here are the test results below:

Test Summary for Linux on commit d16db26

New failed tests found on Linux:

  • menu/view MM-T816 Toggle Full Screen in the Menu Bar

Test Summary for macOS on commit d16db26

New failed tests found on macOS:

  • copylink MM-T125 Copy Link can be used from channel LHS

@devinbinnie devinbinnie merged commit 14bb75e into master Oct 25, 2024
14 of 19 checks passed
@devinbinnie devinbinnie deleted the MM-61271 branch October 25, 2024 13:35
@mattermost-build
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit that referenced this pull request Oct 25, 2024
* [MM-61271] Upgrade to Electron v33.0.2

* Fix node-abi

* Fix and patch nan

(cherry picked from commit 14bb75e)
@mattermost-build mattermost-build added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 25, 2024
devinbinnie added a commit that referenced this pull request Oct 25, 2024
* [MM-61271] Upgrade to Electron v33.0.2

* Fix node-abi

* Fix and patch nan

(cherry picked from commit 14bb75e)

Co-authored-by: Devin Binnie <52460000+devinbinnie@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants