Skip to content

Use @MainActor instead of DispatchQueue.main.async - #1055

Merged
tomholub merged 2 commits into
masterfrom
feature/issue-997
Nov 22, 2021
Merged

Use @MainActor instead of DispatchQueue.main.async#1055
tomholub merged 2 commits into
masterfrom
feature/issue-997

Conversation

@ivan-ushakov

@ivan-ushakovivan-ushakov commented Nov 20, 2021•
edited by sosnovsky
Loading

Copy link
Copy Markdown
Contributor

close#997

Tests(delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

ivan-ushakov
ivan-ushakov commented Nov 20, 2021
View reviewed changes
Comment threadFlowCrypt/Common UI/AttachmentManager.swift
ivan-ushakov
ivan-ushakov commented Nov 20, 2021
View reviewed changes
Comment threadFlowCrypt/Controllers/Compose/ComposeViewController.swift
ivan-ushakov
ivan-ushakov commented Nov 20, 2021
View reviewed changes
Comment threadFlowCrypt/Controllers/Inbox/InboxViewController.swift
ivan-ushakov
ivan-ushakov commented Nov 20, 2021
View reviewed changes
Comment threadFlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift
tomholub
tomholub reviewed Nov 20, 2021
View reviewed changes
Comment threadFlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift
sosnovsky
sosnovsky approved these changes Nov 22, 2021
View reviewed changes

@sosnovskysosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Works well ✅

Comment threadFlowCrypt/Common UI/AttachmentManager.swift
@@ -49,13 +50,19 @@ extension AttachmentManager: AttachmentManagerType {
.sinkFuture(

@sosnovskysosnovskyNov 22, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@tomholub I think we should use async/await here instead of Combine for code consistency across project, and for better threads handling with @MainActor, can be done in another issue

@tomholubtomholubNov 22, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agree - I've asked @ekievsky to do it as part of #1031

@tomholubtomholubNov 22, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Will also file another issue to look over the remaining ones after that, if any

Comment threadFlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift
@tomholub

tomholub commented Nov 22, 2021

Copy link
Copy Markdown
Collaborator

Then I'll merge this, and we make sure that we use actor for Gmail/IMAP/SMTP in another issue.

@tomholubtomholub mentioned this pull request Nov 22, 2021
Closed
@tomholub
tomholub enabled auto-merge (squash) November 22, 2021 10:55
@tomholubtomholub mentioned this pull request Nov 22, 2021
Closed
@tomholub
tomholub merged commit 6f6ec03 into masterNov 22, 2021
@tomholub
tomholub deleted the feature/issue-997 branch November 22, 2021 11:48
tomholub added a commit that referenced this pull request Nov 22, 2021
@tomholub
Revert "issue#997Use@mainactorinstead of DispatchQueue.main.async (…
9c02eeb
…#1055)"
This reverts commit 6f6ec03.
@tomholubtomholub mentioned this pull request Nov 22, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomholubtomholubtomholub approved these changes
@sosnovskysosnovskysosnovsky approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

use @MainActor instead of DispatchQueue.main.async

3 participants

@ivan-ushakov@tomholub@sosnovsky