-
Notifications
You must be signed in to change notification settings - Fork 563
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
Improve auto updater #869
Improve auto updater #869
Conversation
Everything seemed to work OK for me until I tried the production testing. The app never updated on macOS. I got the prompt but the update didn't appear to install. @adlk How can I get access to an AppVeyor build to test on Windows? |
const { dialog } = require('electron'); | ||
const { autoUpdater } = require('electron-updater'); | ||
const sanitizeHtml = require('sanitize-html'); | ||
const TurndownService = require('turndown'); |
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.
I wondered if we could do the HTML to MD conversion with showdown
(the MD converter we're already using), but it looks like the feature hasn't shipped yet 🤷♀️ We'll replace it when it's released.
Are these the correct artifacts for Windows? Production tests:
|
Ah this must have been my issue, I had it in a subfolder in |
@mirka @roundhill thanks for your review. I'll check your issues to figure out what's going on. In the meantime, here are a few answers:
yep, that's the one.
The updater now prompts as expected, for whatever reasons, the windows config + artifacts were missing on the test repo.
👍 In general, I'm a big fan of giving the user more feedback of what's happening – even if it's just a simple spinner. IMO, Github for Desktop does a great job (even though they just inform the user about the update once it's already downloaded)
🤦♂️ This is actually a stupid thinking error I've made. As the electron-updater checks Github for new releases. Everything that is using the manual updater (linux) is still accessing the old API. |
The white screen of death is fixed in #885 |
@mirka @roundhill I've now added support for the manual updater and did a refactor of the updater code base as this will be used for Together with Lena, we've decided to move away from parsing the changelog and showing it in the popup. Instead, we'll show a button to open the changelog. |
Thank you @adlk ! Production build testsUpdated artifacts for testinghttps://circleci.com/gh/Automattic/simplenote-electron/703#artifacts/containers/0 |
@adlk I was able to confirm it working on Ubuntu! Thanks. |
Co-Authored-By: adlk <stefan@adlk.io>
This reverts commit 0edacd0.
c38298b
to
b6affad
Compare
This PR replaces the built in electron auto updater with
electron-updater
. One of the major benefits is added support for windows updates (nsis) and works perfectly in sync with theelectron-builder
binaries.We also don't longer need to maintain a custom API to manage releases. Releases are now handled directly via Github releases that are automatically created whenever a new tag is pushed.
How to do dev/ux-testing
1.2.0
dev-app-update.yml
with the following content:make start
– assuming you have already built the web app once. Otherwise, runmake dev
Please keep in mind that you can't actually update the app as you are running a local and unsigned binary of electron. So in any case, you'll most likely get an error when closing the update dialog. Even though we are in dev mode, all status events will be triggered.
How to do "production" testing
CircleCI > Workflows > Automattic > simplenote-electron > test/auto-updater (latest build) > artifacts (job) > artifacts
/Applications
More infos about electron-updater can be found here.