-
Notifications
You must be signed in to change notification settings - Fork 79
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
Remove unnecessary play services dependency #476
Conversation
@@ -63,7 +63,6 @@ dependencies { | |||
implementation 'androidx.appcompat:appcompat:1.4.0-alpha03' | |||
implementation 'androidx.cardview:cardview:1.0.0' | |||
|
|||
implementation 'com.google.android.gms:play-services-wallet:16.0.1' |
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.
Is this worthy of a CHANGELOG entry or not something that merchants would notice?
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.
They shouldn't notice it - we were duplicate including this dependency (already bundled with the core Google Play module) so it was already resolving to the higher version from core
@@ -284,6 +284,7 @@ private GooglePayRequest getGooglePayRequest() { | |||
.setTotalPriceStatus(WalletConstants.TOTAL_PRICE_STATUS_FINAL) | |||
.build()); | |||
googlePayRequest.setEmailRequired(true); | |||
googlePayRequest.setCountryCode("BR"); |
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.
qq: Noticed this is set to Brazil?
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.
Yeah in prod there's currently an error with our default allowed card networks (I think Maestro or some other one requires the country code to be BR) - updated this rather than changing allowed card networks
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.
Oh ok yeah that makes total sense 👍
Summary of changes
Checklist
[ ] Added a changelog entryAuthors