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

Add the zip target to fix Desktop auto updates #7989

Merged
merged 1 commit into from
Mar 3, 2022

Conversation

kidroca
Copy link
Contributor

@kidroca kidroca commented Mar 3, 2022

@roryabraham

Details

When mac.target was introduced to the electron config it overrode the default, which used to build a dmg and a zip for the current platform
We need to specify a zip target, because autoupdates work with that:

https://www.electron.build/auto-update.html#quick-setup-guide

zip target for macOS is required for Squirrel.Mac, otherwise latest-mac.yml cannot be created, which causes autoUpdater error. Default target for macOS is dmg+zip, so there is no need to explicitly specify target.

Fixed Issues

$ #7954
$ #7987

Tests

  • Verify building Desktop still works
  • Verify it produces zips for each platform in desktop-build

PR Review Checklist

Contributor (PR Author) Checklist

  • I made sure to pull main before submitting my PR for review
  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I clearly indicated the environment tests should be run in (Staging vs Production)
  • I wrote testing steps that cover success & fail scenarios (if applicable)
  • I ran the tests & they passed on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified there are no console errors related to changes in this PR
  • I followed proper code patterns (see Reviewing the code)
    • I added comments when the code was not self explanatory
    • I put all copy / text shown in the product in all src/languages/* files (if applicable)
    • I followed proper naming convention for platform-specific files (if applicable)
    • I followed style guidelines (in Styling.md) for all style edits I made
  • I followed the guidelines as stated in the Review Guidelines

PR Reviewer Checklist

  • I verified the Author pulled main before submitting the PR
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the testing environment is mentioned in the test steps
  • I verified testing steps cover success & fail scenarios (if applicable)
  • I verified tests pass on all platforms & I tested again on all platforms
  • I checked that screenshots or videos are included for tests on all platforms
  • I verified there are no console errors related to changes in this PR
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified comments were added when the code was not self explanatory
    • I verified any copy / text shown in the product was added in all src/languages/* files (if applicable)
    • I verified proper naming convention for platform-specific files was followed (if applicable)
    • I verified style guidelines were followed
  • I verified that this PR follows the guidelines as stated in the Review Guidelines

QA Steps

  • Verify Desktop auto update works. Opening the previous (staging) version should automatically update to latest
  • Verify .dmg download links work (staging): https://staging.new.expensify.com/NewExpensify.dmg and return the installer for the latest version

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

Screenshot 2022-03-03 at 20 51 42

iOS

Android

https://www.electron.build/auto-update.html#quick-setup-guide
> zip target for macOS is required for Squirrel.Mac, otherwise latest-mac.yml cannot be created, which causes autoUpdater error. Default target for macOS is dmg+zip, so there is no need to explicitly specify target.
@kidroca kidroca requested a review from a team as a code owner March 3, 2022 18:52
@MelvinBot MelvinBot requested review from neil-marcellini and removed request for a team March 3, 2022 18:52
@kidroca kidroca changed the title [No QA] Add the zip target to fix Desktop auto updates Add the zip target to fix Desktop auto updates Mar 3, 2022
@kidroca
Copy link
Contributor Author

kidroca commented Mar 3, 2022

This PR does not need to be tested on all platforms - the desktop build config change made, could not in any way affect other platforms

Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

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

It sounds like you did your research and this should fix it. The desktop build worked for me and created the zips. I'll let @roryabraham approve since he is more familiar with the desktop build.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@roryabraham
Copy link
Contributor

Let's see if this works – if not, could we maybe just remove target altogether? I don't recall why we had to add that.

@roryabraham roryabraham merged commit 563d85c into Expensify:main Mar 3, 2022
OSBotify pushed a commit that referenced this pull request Mar 4, 2022
@roryabraham
Copy link
Contributor

roryabraham commented Mar 4, 2022

Downloading from https://staging.new.expensify.com/NewExpensify.dmg seemed to work 🤞

Edit: Nope, still stuck on 1.1.40-2 it seems.

@OSBotify
Copy link
Contributor

OSBotify commented Mar 4, 2022

🚀 Cherry-picked to staging by @roryabraham in version: 1.1.41-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@kidroca kidroca deleted the kidroca/fix/desktop-updates branch March 4, 2022 01:46
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @francoisl in version: 1.1.41-6 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

4 participants