Inform user when checking update fail #245
Conversation
…de in one try catch instead of catch each exception seperately since they are all same kind of exception. Use using instead of manually dispose.
Flow.Launcher.Core/Updater.cs
Outdated
| { | ||
| Log.Exception($"|Updater.UpdateApp|Check your connection and proxy settings to github-cloud.s3.amazonaws.com.", e); | ||
| updateManager.Dispose(); | ||
| api.ShowMsg("Update Fail!", "Check your connection and proxy settings to github-cloud.s3.amazonaws.com."); |
There was a problem hiding this comment.
just wording while we are at it, can we update title to "Updated failed" and subtitle to "Check your connection and try updating proxy settings to github-cloud.s3.amazonaws.com."
Flow.Launcher.Core/Updater.cs
Outdated
| { | ||
| if (!silentUpdate) | ||
| MessageBox.Show("You already have the latest Flow Launcher version"); | ||
| updateManager.Dispose(); |
There was a problem hiding this comment.
since it's using using var updateManager = await GitHubUpdateManager(GitHubRepository); in the above code, this Dispose() is not needed right?
Flow.Launcher.Core/Updater.cs
Outdated
| if (!silentUpdate) | ||
| api.ShowMsg("Please wait...", "Checking for new update"); | ||
|
|
||
| using var updateManager = await GitHubUpdateManager(GitHubRepository); |
There was a problem hiding this comment.
back when i was working on this section of the code i read Squirrel had some reported issues where it is not properly disposing the updateManager, could we double check this issue is no longer relevant in our Squirrel version please. I think it was to do with certain Squirrel versions and when this project was still on .Net Framework
There was a problem hiding this comment.
I remember there is a comment on this part of code that said we need to manually dispose the updatemanager for squirrel 1.5.2, but the use of using will be fine since it is equal to dispose at the end of the scope.
jjw24
added
enhancement
just logging the update failure is definitely not cool, user should know when update fails for sure |
Agree with you! But since in my condition, the checking fails every time..... Let's also take a look at #171 |
|
could you please do a merge dev on this one. also per comment above (line 56 of Updater.cs) do we need dispose still? |
Sure |
…Launcher into InformUserWhenUpdateFail
Removed |
jjw24
removed
the
review in progress
2 participants
close #168
Will this be annoying if flow fails checking update every time?