GoogleSignIn-iOS icon indicating copy to clipboard operation
GoogleSignIn-iOS copied to clipboard

Make asynchronous methods callable via async/await

Open peterfriese opened this issue 1 year ago • 6 comments

This fixes #91

@mdmathias @petea - can you provide guidance on formatting / indentation? I will also look into updating the sample as part of this PR.

peterfriese avatar Jul 17 '22 18:07 peterfriese

Hi @peterfriese thanks for the PR! It looks like it fails some checks during pod lib lint on macOS 10.15.

I was thinking that we'd address #91 by updating the block parameter names to be completion.

mdmathias avatar Jul 19 '22 16:07 mdmathias

Let me look into the lint checks.

I was thinking about renaming the block parameter to completion as well at first, but was worried that this would be a breaking change for the API. What are your thoughts about this?

peterfriese avatar Jul 19 '22 16:07 peterfriese

You're correct - changing the block parameter name to completion would break the API and require a major version release. Does adding the swift_async_name attribute not break the API? I guess it just adds an async version of the method and doesn't replace the pre-existing version. I'm not as familiar with with this attribute...

In any case, if swift_async_name doesn't break the API, and only adds async methods, then I think it's worth figuring out what is causing issues in the linting.

mdmathias avatar Jul 20 '22 17:07 mdmathias

Another consideration is that we are due to have a major release coming up soon anyway. Perhaps it's okay to change this API? What do you think @petea?

mdmathias avatar Jul 20 '22 21:07 mdmathias

I did a little informal test and it looks like adding the swift_async_name attribute just adds an async method, but leaves the existing API unchanged.

Without the attribute: Screen Shot 2022-07-20 at 2 59 02 PM

With attribute: Screen Shot 2022-07-20 at 3 04 45 PM

mdmathias avatar Jul 20 '22 22:07 mdmathias

@peterfriese thanks for bringing swift_async_name to our attention. This could allow us to ship Swift Concurrency support in a non-breaking fashion. However, we are planning a major release toward the end of Q3 and it may be preferable to take that opportunity to rename callback -> completion for automatic Swift async method generation as well as slightly more idiomatic method naming in general. In either case, swift_async_name might be useful for doWithFreshTokens: in GIDAuthentication, as this method doesn't even begin to conform to the async naming conventions.

petea avatar Jul 21 '22 20:07 petea

Addressed with #187 and will go out in our next major release.

petea avatar Sep 07 '22 23:09 petea