Add Galaxy Store wrapper dependency for hybrid SDKs#1673
Conversation
28b0ada to
114587a
Compare
114587a to
3e022d5
Compare
3e022d5 to
f7d479b
Compare
tonidero
left a comment
There was a problem hiding this comment.
I think this makes sense!
| } | ||
|
|
||
| android { | ||
| namespace = "com.revenuecat.purchases.hybridcommon.galaxy" |
There was a problem hiding this comment.
Supernitpicky... I wonder if we should make the namespace com.revenuecat.purchases.galaxy.hybridcommon... but not a strong opinion. I think it's probably fine as is.
| } | ||
|
|
||
| dependencies { | ||
| api(project(":hybridcommon")) |
There was a problem hiding this comment.
Considering there is no code, we might not need this... (this means we will bring the main SDK along when adding the galaxy SDK). But having said that, I don't think it's a big deal to do so either so can leave as is.
| diagnosticsEnabled: Boolean? = null, | ||
| automaticDeviceIdentifierCollectionEnabled: Boolean? = null, | ||
| preferredLocale: String? = null, | ||
| galaxyBillingMode: String? = null, |
There was a problem hiding this comment.
Hmm I'm thinking whether it could be worth to add something more extensible like galaxyConfigurationOptions which is a map of string to string. So in case we add more galaxy-only options, we don't need to add it to the configure method which is getting unwieldly 😅. TBH, if we wanted to make it really easy in the hybrids (at the cost of any compilation safety), would be to make a single map to any parameter containing all parameters... But that's a separate discussion 😅.
So happy to leave as this for now and we can think about this refactor separately.
| null, "PRODUCTION" -> galaxyBillingMode(GalaxyBillingMode.PRODUCTION) | ||
| "TEST" -> galaxyBillingMode(GalaxyBillingMode.TEST) | ||
| "ALWAYS_FAIL" -> galaxyBillingMode(GalaxyBillingMode.ALWAYS_FAIL) | ||
| else -> warnLog("Attempted to configure with unknown Galaxy billing mode: $galaxyBillingMode.") |
There was a problem hiding this comment.
Should we also default to production in this case? Not sure if it's needed to set it though...
| project_dir: 'android' | ||
| ) | ||
|
|
||
| # Publish hybridcommon-store-galaxy with -bc7 suffix |
There was a problem hiding this comment.
TBH, this is likely not needed... We actually should remove it soon (Google Play won't accept updates with BC7 soon). So feel free to leave it out.
|
|
||
| buildTypes { | ||
| release { | ||
| isMinifyEnabled = false |
There was a problem hiding this comment.
Hmm I would say probably not needed? or it could be set to true I think? I see we do have it set to false in the "main" hybridcommon library... But I think it's not a good choice... Probably each hybrid will need to reference the necessary methods, and it's there where we might need to set minification to false (not sure TBH)
| @@ -0,0 +1,65 @@ | |||
| @Suppress("DSL_SCOPE_VIOLATION") // TODO: Remove once KTIJ-19369 is fixed | |||
There was a problem hiding this comment.
I believe this might have been fixed in gradle 8.1? If so, we might not need it.
Summary
This PR adds the
purchases-hybrid-common-store-galaxyadd-on dependency that allows simplified bringing Galaxy Store support to the hybrid SDKs.Motivation
The addition of the wrapper dependency simplifies dependency version management by allowing the hybrid SDKs to just depend on another PHC library (next to
purchases-hybrid-commonand optionallypurchases-hybrid-common-ui) using the same PHC version number.The newly added store support wrapper library (
purchases-hybrid-common-store-galaxy) will in turn depend on the right version (the same version as the core dependency) of the native store support dependency (purchases-store-galaxy). This ensures simple version management and avoids the hybrid SDKs having to depend on a specific version of the native store support dependency as well as the matching version of PHC.Dependency visualization
Testing