refactor: remove deprecated apis from brownfield-gradle-plugin#367
Conversation
…nto feat/revamp-plugin
…nto refactor/bgp/remove-deprecated-apis
hurali97
changed the title
| //tasks.named("detekt").configure { | ||
| // dependsOn(":ktlintFormat") | ||
| //} | ||
| // | ||
| //tasks.register("lint") { | ||
| // dependsOn(":ktlintFormat") | ||
| //} |
There was a problem hiding this comment.
Are you sure this is to remain?
There was a problem hiding this comment.
Ah no 😄 Self review of this PR is pending, will do it Monday!
| androidComponent?.buildTypes?.forEach { | ||
| if (it.name == variant.name || it.name == variant.buildType) { | ||
| if (androidComponent.productFlavors.isEmpty()) { | ||
| taskName = it.name.capitalized() | ||
| return@forEach |
There was a problem hiding this comment.
The old implementation matched dependency variants using missingDimensionStrategies fallbacks. With product flavors, the code now always picks the first flavor, not the flavor for the current variant. Are we sure this is the best way to go?
There was a problem hiding this comment.
The reason of removing the old implementation is that it was never used.
The legacy implementation was there to handle the cases where a third party library could define different flavors, dimensions and missingDimensionsStrategy. However, most react native libraries do not define extra flavors and dims other than the stock release and debug ones. The missingDimensionsStrategy would still need to be defined on the third party library level and I am not aware of any library followed this.
In a way, most public packages do not define the flavors on their libraries as it is then a responsibility of the consumer :app to define the missing dimension. This is a classic way even without brownfield.
However, since we know some people like to do things differently, we still support missing dimensions but they have to be defined on the :brownfield module like:
missingDimensionStrategy(dimToUse, flavorToUse)
Verified this flow by adding flavor and dimension to brownfield-navigation and it works as expected.
However, I would like to address your next point below, which changes things a bit!
With product flavors, the code now always picks the first flavor, not the flavor for the current variant.
Thanks for pointing this out. However a subtle correction, the code always picks the last flavor, regardless this is an incorrect behavior as we do not know which flavor to pick.
With the new LibraryExtension API, we do not have a way to know what missingDimensionStrategy the consumer project defined. To allow BGP to know what missing strategy to use, we expose an extension property:
reactBrownfield {
missingDimensionStrategies = listOf(dimToUse, flavorToUse)
}Then we read it during our logic and pick the matching one. If no match is found, we throw an error!
See the updated code, thanks!
|
|
||
| return matchedVariant?.let { variantTaskProvider.bundleTaskProvider(project, it.name) } | ||
| } | ||
| return variantTaskProvider.bundleTaskProvider(project, taskName) |
There was a problem hiding this comment.
Are we sure this will be non-null at all times? Don't we want to use optional chaining + optional task as we used to?
There was a problem hiding this comment.
Yes, because the previous approach used .find which can be null, so we had to use optionality check, now we always have a task name.
…nto refactor/bgp/remove-deprecated-apis
artus9033
left a comment
artus9033
left a comment
There was a problem hiding this comment.
LGTM after 1 nit comment!
Summary
This replaces the deprecated APIs like
LibraryVariant, legacy DSL and others to instead use the latest ones. This is required to ensure a smooth adoption once AGP v9 lands.Also tested this PR locally with RNApp enabling AGP v9 and it works well 🚀
Test plan