-
Notifications
You must be signed in to change notification settings - Fork 629
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
Update compose and kotlin. #7718
Update compose and kotlin. #7718
Conversation
Diffuse output:
APK
DEX
|
kotlinCoroutines : '1.6.4', | ||
kotlinSerialization : '1.5.0', | ||
kotlin : '1.9.21', | ||
kotlinCoroutines : '1.7.3', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should update playServices
to the same version.
f2214ba
to
308d91b
Compare
308d91b
to
ffaab7f
Compare
bbf479c
to
c0cdc3b
Compare
@@ -31,11 +31,9 @@ abstract class PaymentAuthenticator<Authenticatable> : ActivityResultLauncherHos | |||
requestOptions: ApiRequest.Options | |||
) { | |||
val lifecycleOwner = host.lifecycleOwner | |||
lifecycleOwner.lifecycleScope.launch { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior change might not be intentional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave this a quick test run and it seems to work fine when running in the existing scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same! I also ran through the code to make sure it was logical. But I think this was really the only "change", so figured I'd call it out.
Changes LGTM, since both Till and I contributed here. |
LGTM! |
Summary
Motivation
Testing
Screenshots
Changelog