Skip to content

Upgrade Gradle to 7.4 & AGP to 7.1.1#55

Merged
ParaskP7 merged 27 commits intotrunkfrom
update/gradle-to-7.3.3-agp-to-7.0.4
Mar 23, 2022
Merged

Upgrade Gradle to 7.4 & AGP to 7.1.1#55
ParaskP7 merged 27 commits intotrunkfrom
update/gradle-to-7.3.3-agp-to-7.0.4

Conversation

@ParaskP7
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 commented Mar 10, 2022

This PR upgrades Gradle to 7.4 & AGP to 7.1.1.

It includes the following:

  • Gradle version upgraded to 7.3.3 with the ./gradlew wrapper --gradle-version=7.3.3 --distribution-type=all command.
  • AGP version upgrade to 7.0.4 (see settings.gradle change).
  • Build output diff:
    • To identify and fix new warnings/errors that got added.
    • To identify and verify old warnings/errors that got removed.
  • Lint output diff
    • To identify and fix new warnings/errors that got added.
    • To identify and verify old warnings/errors that got removed.
  • The above 4 steps were redone with a follow-up update to Gradle 7.4 & AGP 7.1.1.
  • Update buildkite docker android build image to 1.2.0

It suppresses the following Lint issues:

It resolves the following Lint issues:

It enables Lint warnings as errors for the following modules:

It disables the following Lint issues:


Besides the above, you can reference the Gradle 7.4 & AGP 7.1.1 PRs of the below clients for testing/verification purposes:

NOTE: This is a draft PR because all Gradle & AGP upgrade PRs need to be merged together.

Merge Instructions

To Test

  1. Smoke test the sample app.
  2. CI checks will cover everything in terms of testing.
  3. Testing composite builds and respective clients will be enough (see above).

ParaskP7 added 17 commits March 9, 2022 17:11
Task failure message:
"> Task :mediapicker:source-gif:processDebugAndroidTestManifest FAILED
.../mediapicker/source-gif/build/intermediates/tmp/manifest/androidTest
/debug/tempFile1ProcessTestManifest123.xml:5:5-72 Error:
 uses-sdk:minSdkVersion 1 cannot be smaller than version 21 declared in
 library [:mediapicker:domain]
 .../mediapicker/domain/build/intermediates/merged_manifest
 /debug/AndroidManifest.xml
 as the library might be using APIs not available in 1
  Suggestion: use a compatible library with a minSdk of at most 1,
   or increase this project's minSdk version to at least 21,
   or use tools:overrideLibrary="org.wordpress.android.mediapicker.api"
   to force usage (may lead to runtime failures)"
This change suppressed both, the 'VectorRaster' and 'UnusedAttribute'
Lint warnings for this drawable.
Also, as part of this change the previously disabled
'ObsoleteLintCustomCheck' error is removed as it is no longer needed.
@ParaskP7 ParaskP7 added the enhancement New feature or request label Mar 10, 2022
@ParaskP7 ParaskP7 requested a review from oguzkocer March 10, 2022 09:47
@ParaskP7 ParaskP7 self-assigned this Mar 10, 2022
…cker-Android into update/gradle-to-7.3.3-agp-to-7.0.4

� Conflicts:
�	settings.gradle
This is done because otherwise some WCAndroid client tests fail with
an 'advanceUntilIdle' and/or 'advanceTimeBy' unresolved reference error.
@ParaskP7 ParaskP7 marked this pull request as ready for review March 18, 2022 17:16
@ParaskP7 ParaskP7 requested review from a team and removed request for oguzkocer March 18, 2022 17:16
appcompatVersion = '1.4.1'
coreVersion = '1.7.0'
coroutinesVersion = '1.6.0'
coroutinesVersion = '1.5.2'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised by this, then I saw the commit message explaining why ( ❤️ ):

This is done because otherwise some WCAndroid client tests fail with an 'advanceUntilIdle' and/or 'advanceTimeBy' unresolved reference error.

Copy link
Copy Markdown
Contributor Author

@ParaskP7 ParaskP7 Mar 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @mokagio ! 🙏

I was surprised by this, then I saw the commit message explaining why ( ❤️ ):

Yes, @0nko made this upgrade on the previous #56 PR, but as it seems it is actually incompatible with WCAndroid, the WCAndoird Coroutines version, which currently is the 1.5.2. For some reason, the newest 1.6.0 version, comming from the library, overrides the one from the client. Then, this creates test failures as the newer version of Coroutines there were lots of major testing related changes that when upgrading need to be handled with care.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I've been trying to smoke test the app locally, but I keep having Emulator issues etc. By now it's become personal, so I'll have to get to the bottom of it even if someone else reviews it before me.

Besides, while I'm confident in going through the sample app in monkey mode and in verifying WooCommerce Android builds when using the local composite build version of this, I don't have much to add when it comes to the technicalities of the upgrade. It's still best for someone that actually knows Android to look over this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 @mokagio !

Thanks for trying to smoke test the app! 🙏

BTW, I've been trying to smoke test the app locally, but I keep having Emulator issues etc.

Having fun with Emulators... 🤗 What kind of issues are you getting, maybe I can try to help?

By now it's become personal, so I'll have to get to the bottom of it even if someone else reviews it before me.

😅 Good luck, I hope you win this battle! 🤞

Besides, while I'm confident in going through the sample app in monkey mode and in verifying WooCommerce Android builds when using the local composite build version of this, I don't have much to add when it comes to the technicalities of the upgrade.

👍

It's still best for someone that actually knows Android to look over this.

I hear you, maybe I can drag @0nko here to help with a quick review, as always through shameless pinging... 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked the code and tested the sample app. Everything is looking good 👍.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for checking and testing this PR @0nko ! 🙇

@oguzkocer oguzkocer self-requested a review March 22, 2022 16:19
@ParaskP7 ParaskP7 merged commit d0db263 into trunk Mar 23, 2022
@ParaskP7 ParaskP7 deleted the update/gradle-to-7.3.3-agp-to-7.0.4 branch March 23, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants