This repository has been archived by the owner on Nov 2, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 110
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
bb972c2
commit 14fd0c3
Showing
1 changed file
with
3 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14fd0c3
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.
Ffs, this could have just had the api call changed. Or just remove the
apiCall()
if '/daemon/updates/check' isn't an endpoint anymore. This is now nonsensical and there's a useless button based on it in the top-right14fd0c3
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.
This was pushed directly to master? =/
14fd0c3
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.
Does electron support in-place updating?
14fd0c3
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.
Why would a user need to update in place? I can't think of many applications that do that.
14fd0c3
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 guess restarting is fine. I just don't want people to have to be copying folders and consensus data and whatnot
14fd0c3
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.
That's necessarily going to require an installer executable, right? Isn't that how every other app works?
14fd0c3
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.
'siad' is able to upgrade in-place, which means users never have to touch or copy the files.
14fd0c3
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.
That's because it's one file executable, I'm not sure of how we could get Sia UI to that level.
I suppose that with a separately located siad folder, the UI just needs to maintain the config.json that points to it. Then the UI could be installed over without manual copying of the siad files. However this requires a settings page
14fd0c3
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.
The real issue here was that the existing code wasn't handling the error condition properly. Instead of setting
Running
to false and trying again later, it should have just silently done nothing.And fwiw, the current update system is deprecated, so the update dialog wasn't serving much purpose anyway.