Skip to content

Conversation

gave92
Copy link
Member

@gave92 gave92 commented Oct 31, 2022

Resolved / Related Issues
Items resolved / related issues by this PR.

  • Fix crash when checking for updates. IUpdateService methods are supposed to be called from the UI thread.
  • Wait for file transfers to complete before terminating process
  • Restored call to UpdateTagsDb()

Validation
How did you test these changes?

  • Built and ran the app

@yaira2 yaira2 requested a review from hez2010 October 31, 2022 21:22
@yaira2 yaira2 changed the title Fix crash when checking for updates Fixed crash that would occur when checking for updates Oct 31, 2022
@hez2010
Copy link
Member

hez2010 commented Nov 1, 2022

I don't think all code in InitializeAppComponentsAsync should run on the UI thread. This will make the window slower to show.
How about calling IUpdateService only on the UI thread? i.e. DispatcherQueue.TryEnqueue(check for updates)

@yaira2
Copy link
Member

yaira2 commented Nov 1, 2022

The check for update code could also be deferred until everything else is loaded.

@gave92
Copy link
Member Author

gave92 commented Nov 1, 2022

The code in InitializeAppComponentsAsync() is not called all on ui thread. There's a Task.Run in there. Update check is deferred untill everything else is loaded.

Copy link
Member

@hez2010 hez2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Nov 1, 2022
@yaira2 yaira2 merged commit 26b378c into files-community:main Nov 1, 2022
@gave92 gave92 deleted the update branch November 1, 2022 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants