-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Desktop: Seamless-Updates - creation of auto updater service #10772
Conversation
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.
Thanks Alice, that looks good, I've just left a few comments
| @@ -0,0 +1,120 @@ | |||
| import { app, BrowserWindow } from 'electron'; | |||
| import { autoUpdater, UpdateInfo } from 'electron-updater'; | |||
| import log from 'electron-log'; | |||
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.
If possible please use our Logger class, so that log statements are sent to the expected file. Have a look how it's done in other files, but the pattern basically is const logger = Logger.create('AutoUpdaterService')
| import { autoUpdater, UpdateInfo } from 'electron-updater'; | ||
| import log from 'electron-log'; | ||
| import path = require('path'); | ||
| import { setInterval } from 'timers'; |
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.
Please use shim.setInterval if possible - the reason is that it's going to use a more reliable timer and it's also easier to track all timers in the app.
| const DEFAULT_UPDATE_INTERVAL = 12 * 60 * 60 * 1000; | ||
| const INITIAL_UPDATE_STARTUP = 5 * 1000; |
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.
We use camelCase for constants: https://joplinapp.org/help/dev/coding_style#use-camelcase-for-constants-in-new-code
|
Thanks Alice, let's merge this! |
This PR contains the Auto Updater Service. The service will only log the update events and will not download or install the updates (for now). This way, this PR's code can monitor the availability of updates through the logs without triggering any update process.
Events that are logged: