Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve TODOs by backporting async/await code #610

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Conversation

jkmassel
Copy link
Contributor

Description

While digging into async issues, I came across a couple of TODOs that were pretty quick to clean up. While we can't switch to async/await entirely at this point, enabling backporting is a step toward allowing it (even if just for use in our test suite).

Testing Details

Ensure tests pass.


  • Please check here if your pull request includes additional test coverage.
  • I have considered updating the version in the .podspec file.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

@jkmassel jkmassel added the enhancement New feature or request label Jun 20, 2023
@jkmassel jkmassel self-assigned this Jun 20, 2023
@@ -498,10 +500,23 @@ open class WordPressComRestApi: NSObject {
request.httpBody = parameters.percentEncoded()
}

return try await urlSession.data(for: request, delegate: nil)
return try await withCheckedThrowingContinuation { continuation in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

urlSession.data(for:) is only available in iOS15, so I had to wrap the method in async/await myself

Copy link
Contributor

Choose a reason for hiding this comment

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

Jetpack/WordPress will soon support iOS 15 and above only and WooCommerce already does.

Your diff doesn't add much complexity, but I wonder if it would be cleaner to bump the library to iOS 15 instead.

@jkmassel jkmassel marked this pull request as ready for review June 20, 2023 17:26
@leandroalonso
Copy link
Contributor

@jkmassel it looks like the tests are hanging after testSuccessfullGetCodableCall 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants