fix: backwards compatibility with older AGPs #164
Conversation
* fix: add namespace for AGP <4.2 support * chore: backwards compatibility * fix: rm package from manifest for gradle 8.0
Reverting these as I personally think just setting namespace for all AGPs is a better solution
|
Thank you. Sorry for tangent, it's very hard to communicate as the repo doesn't allow for issues, and instead focus on intercom community forum which adds a lot of friction. I would also make a suggestion of adding CI flows to build the example app and e2e test, even if just app launch success |
No problem at all! I agree, I get why they disable issues, but it does add a ton of friction.
The PR looks fine though, if its installing in that path that the author referenced i think it LGTM. Obviously testing would be a better idea than just looking though ;). Will test it if I get the time! |
| @@ -23,11 +23,7 @@ def safeExtGet(prop, fallback) { | |||
| } | |||
|
|
|||
| android { | |||
| // Compatibility for AGP v. <4.2/Gradle 8 | |||
| def agpVersion = com.android.Version.ANDROID_GRADLE_PLUGIN_VERSION.tokenize('.')[0].toInteger() | |||
There was a problem hiding this comment.
You are removing the declaration of agpVersion which is used at line 50😲
There was a problem hiding this comment.
@Looskie Also on rn versions < 0.70 pod install & android build seems to fail with below error (package version - 6.5.0). I'm not sure where issues for this package are tracked as I don't see any Issues section for this repo.
Following this comment react-native-community/cli#1984 (comment), adding package name property seems to fix the issue
|
I'm marking this a draft for now as I clearly need to test it. Will test when I get the bandwidth |
|
Maybe just merge the namespace? Or that breaks something? @Looskie |
Sorry for the late reply, it would break versions 0.73.0 0.73.1 and 0.73.2. 0.73.3 and higher infer these changes by default for backwards compatibility. I'll make some time this weekend or maybe during the evening to make a permanent solution. |
As pointed out by @filipef101, I accidentally removed the namespace here.
Furthermore, this entire patch for newer AGPs (Including this PR, and #152) is actually not even needed anymore considering RN 0.73.3 (Not 0.73.2/0.73.1 though) adds backwards compatibility for packages to use
namespace. but, it doesn't hurt to still have it!