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-builder #749

Merged
merged 7 commits into from
Apr 6, 2018

Conversation

yuya-oc
Copy link
Contributor

@yuya-oc yuya-oc commented Apr 2, 2018

Before submitting, please confirm you've

Please provide the following information:

Summary
Upgrade electron-builder.

It has an issue about Linux icon. As a workaround, macOS icon is also used for Linux in this PR.

Issue link
#702

Test Cases
Download artifacts. Then, check artifacts name, package name, publisher name and icons.

Additional Notes
https://circleci.com/gh/yuya-oc/desktop/736#artifacts

@jasonblais
Copy link
Contributor

@yuya-oc Looks like only the Zip packages were uploaded on CircleCI:
image

@yuya-oc yuya-oc force-pushed the upgrade-electron-builder branch from b41a5df to 7b80586 Compare April 2, 2018 13:54
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Apr 2, 2018

@jasonblais Thanks to let me know. Artifacts name had been changed by electron-builder. Here is fixed version.

https://circleci.com/gh/yuya-oc/desktop/736#artifacts

@jasonblais
Copy link
Contributor

jasonblais commented Apr 2, 2018

@yuya-oc I'm getting an error when trying to launch the app on Mac (Mattermost 24 is equivalent to Mattermost)

image

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Apr 2, 2018

@jasonblais Reproduced also on local build. Probably it's a bug of electron-builder. (similar issue: electron-userland/electron-builder#784)

But possibly it's good time to move to zip (it works). And it's required for auto-updater.

@jasonblais
Copy link
Contributor

@yuya-oc Will there be any side effects to the user?

Were we able to use the .dmg files for auto-updates, I forget? I think you mentioned the flow for .dmg files would be similar to Windows .zip files? #588

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Apr 2, 2018

@jasonblais I think there are nothing other than filename (of course we should test though).

For mac, dmg and zip are almost same, but they have different purpose in auto-updater context.

  • dmg: For initial installation.
  • zip: Downloaded and applied by updater. It also can be used manually as well as our tar.gz.

@jasonblais
Copy link
Contributor

@yuya-oc So would auto-updates work for .dmg? Or would the user download .dmg initially (if desired), but then .zip files for updates?

@jasonblais
Copy link
Contributor

As for using tar.gz or zip files, I'm okay moving to .zip files if there are no changes to the user. Especially since we'll be changing to it anyways soon.

@amyblais amyblais self-requested a review April 2, 2018 22:06
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Apr 3, 2018

auto-updater would work for both of zip and dmg. But they check whether .zip exists on the server. So their difference is just file format (and some extra pictures for appearance of dmg). I'll prepare zip version.

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Apr 3, 2018

@jasonblais
Copy link
Contributor

@yuya-oc Have confirmed on Mac, no issues found.

@amyblais Please help confirm on Windows?

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Apr 5, 2018

Tested on Ubuntu 16.04. No issue found, and icons are correctly included.

@jasonblais
Copy link
Contributor

Thanks all! @yuya-oc we should be good to merge this one.

@yuya-oc yuya-oc merged commit b522b8d into mattermost:master Apr 6, 2018
@yuya-oc yuya-oc deleted the upgrade-electron-builder branch April 6, 2018 12:40
@yuya-oc yuya-oc mentioned this pull request Apr 6, 2018
10 tasks
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.

3 participants