Skip to content

fix(maker-wix): set default arch - #3859

Merged
erickzhao merged 1 commit into
electron:nextfrom
ShGKme:fix/maker-wix--arch
Dec 10, 2025
Merged

erickzhao merged 1 commit into
electron:nextfrom
ShGKme:fix/maker-wix--arch

Conversation

@ShGKme

@ShGKme ShGKme commented Feb 19, 2025

Copy link
Copy Markdown
Contributor
  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

electron-wix-msi defaults to x86 arch.

Even if @electron-forge is set to make x64 installer, by default x86 installer will be created, and the app will be installed to C:\Program Files (x86)\.

⚠️ Note: this might be considered a breaking change - new installation will be in a new directory (C:\Program Files\ after C:\Program Files (x86)\).

Alternative: maker users can manually pass arch.

@ShGKme
ShGKme requested a review from a team as a code owner February 19, 2025 22:17
erickzhao
erickzhao previously approved these changes Feb 20, 2025

@erickzhao erickzhao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This LGTM. I think you're right about this being a breaking change, but I think building according to the arch makes the most sense according to how we normally output Forge distributables.

I'm a +1 for this, but only merging after we start the Forge 8 release process.

@erickzhao
erickzhao marked this pull request as draft February 20, 2025 01:51
@erickzhao erickzhao added this to the 8.0.0 milestone Feb 20, 2025
@erickzhao
erickzhao changed the base branch from main to next December 5, 2025 20:18
@erickzhao
erickzhao dismissed their stale review December 5, 2025 20:18

The base branch was changed.

@erickzhao
erickzhao marked this pull request as ready for review December 5, 2025 20:19
@erickzhao

Copy link
Copy Markdown
Member

Hey @ShGKme, the Electron org now requires you to sign your commits when making PRs. Could you do that and push up the rebased commits before I merge this PR in?

@ShGKme

ShGKme commented Dec 8, 2025

Copy link
Copy Markdown
Contributor Author

I'll do it first thing in the morning (I'm in Central Europe Time)

@ShGKme
ShGKme force-pushed the fix/maker-wix--arch branch 2 times, most recently from f45c688 to abdc74a Compare December 9, 2025 14:44
@erickzhao

Copy link
Copy Markdown
Member

@ShGKme I think you might be potentially missing a step in the commit signing flow? CI is telling me abdc74a still isn't signed. :(

It could be that the public key isn't set up on your account?

image

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme
ShGKme force-pushed the fix/maker-wix--arch branch from abdc74a to 9750e1e Compare December 9, 2025 18:59
@ShGKme

ShGKme commented Dec 9, 2025

Copy link
Copy Markdown
Contributor Author

@erickzhao Sorry, missed that lazygit doesn't sign on amend if it wasn't signed 👀

@erickzhao
erickzhao enabled auto-merge (squash) December 10, 2025 21:38
@erickzhao
erickzhao merged commit df3127c into electron:next Dec 10, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants