-
Notifications
You must be signed in to change notification settings - Fork 121
Upgrade Gradle to 7.1.1 & Android Gradle Plugin to 4.2.2 #940
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
Conversation
build.gradle
Outdated
| dependencies { | ||
| classpath "com.android.tools.build:gradle:$gradlePluginVersion" | ||
| classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" | ||
| commonTargetSdkVersion = 29 |
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.
Please bump it up to 30, given everything else is upgraded now.
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.
@ravishanker I've left a note for you in the PR description about this:
Updates targetSdkVersion to 29 - this was a necessary change to get the library building with the other changes. We probably want to upgrade it to 30 to match the latest changes in other projects. @ravishanker Could you take a look at that since you have recently worked on this? I am not sure if there is anything that needs to be changed in the library to support that change. This is the only change I am a little bit worried about as I don't have enough knowledge/experience about target sdk changes.
Unfortunately your comment is not clear enough to tell whether you've read the PR description or if there is anything to be considered for upgrading this library's targetSdkVersion.
|
Just bringing a discussion from Slack to here. Before merging this we should ensure that we can still generate artifacts for this library. Currently, we're using Jitpack for that, but the build from this PR failed. You mentioned that you might go ahead and just migrate this from jitpack to s3, which sounds like a fine solution to me. Of course, fixing the Jitpack issue is also fine, but if the migration isn't too hard it might be easier to maintain in the future. No strong preferences from my end though. |
mchowning
left a comment
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.
Changes look good, and with your other PR moving us from jitpack to S3, publishing shouldn't be a problem either.
This PR updates Gradle to 7.1.1 and Android Gradle Plugin to 4.2.2 which matches our major libraries & clients. Here are a few changes that were made as part of this change:
enableUnitTestBinaryResourcesfromgradle.propertiesflag in d9927bd which Gradle said is not supported - and didn't make a differencebuildToolsVersionin fc6eb05 as it's now part of Android Gradle Plugin and has been stripped from our build files for other projects for some time4.4which matches current WPAndroid and fixes an issue with the build f73f1f0targetSdkVersionto 29 - this was a necessary change to get the library building with the other changes. We probably want to upgrade it to 30 to match the latest changes in other projects. @ravishanker Could you take a look at that since you have recently worked on this? I am not sure if there is anything that needs to be changed in the library to support that change. This is the only change I am a little bit worried about as I don't have enough knowledge/experience about target sdk changes..idea/compiler.xmlwhich is something we have done a while back for other projectsThese changes were made with the major goal of supporting composite builds, and minor goal of bringing the library up to speed with other projects. As always, there is still more to be improved.
To test:
Follow the test instructions in wordpress-mobile/WordPress-Android#15432