Skip to content

Replace Promise and Future with async/await - #689

Merged
tomholub merged 1 commit into
masterfrom
feature/issue-404
Oct 18, 2021
Merged

Replace Promise and Future with async/await#689
tomholub merged 1 commit into
masterfrom
feature/issue-404

Conversation

@ivan-ushakov

Copy link
Copy Markdown
Contributor

close#404


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 marked this pull request as draft October 17, 2021 13:11
@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

Will do this step by step to avoid a lot of changes in one PR (hard to review).

@tomholub

Copy link
Copy Markdown
Collaborator

Let me know once it's ready for merging.

Comment on lines +17 to 20
var messages: [Message] = []
for identifier in messageIdentifiers {
messages.append(try await fetchFullMessage(with: identifier))
}

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.

In a separate issue, should make sure these pull in parallel instead of sequentially.


return Promise { resolve, reject in
return try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<GTLRGmail_ListMessagesResponse, Error>) in
self.gmailService.executeQuery(query) { _, data, error in
if let error = error {
reject(GmailServiceError.providerError(error))
continuation.resume(throwing: GmailServiceError.providerError(error))
return
}

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.

I suppose this is because self.gmailService.executeQuery is not async yet?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

gmailService has type GTLRService and this is third party code in objc. We could create Swift wrapper with async methods but not sure this worth it.

@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

Let me know once it's ready for merging.

I think it is ready. We could do multiple PR for #404 but close in first message looks confusing. Maybe we could write partially close.

@ivan-ushakov
ivan-ushakov marked this pull request as ready for review October 18, 2021 18:21
@tomholub
tomholub merged commit f7bb5f0 into masterOct 18, 2021
@tomholub
tomholub deleted the feature/issue-404 branch October 18, 2021 18:44
@tomholub

Copy link
Copy Markdown
Collaborator

Let me know once it's ready for merging.

I think it is ready. We could do multiple PR for #404 but close in first message looks confusing. Maybe we could write partially close.

When you don't want to close an issue, write issue #nnn instead of close #nnn in the issue description.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace Promise with async/await

2 participants

@ivan-ushakov@tomholub