[Deps] Migrate to Version Catalogs #12765
Conversation
FYI: Notice that the Japanese, Chinese and Korea 'Text Recognition' related dependencies are not 'Google MLKit' related but actually 'Google Play Services' related. However, to keep things simple and since the version is coming from 'Google MLKit' and it main 'Text Recognition' dependency the overall version catalog is pointing to 'Google MLKit' and not 'Google Play Services'.
FYI: Notice that the 'AndroidX Core Splashscreen' version on the 'WooCommerce' module is pointing to '1.0.0', while on the 'WooCommerce-Wear' module it is pointing to '1.0.1'. This change also updates the 'WooCommerce' version to point to '1.0.1' as well, it being just a batch update that includes a single bugfix and no more. Release Notes: https://developer.android.com/jetpack/androidx/releases/ core#core-splashscreen-1.0.1
FYI: Notice that the 'AndroidX Preference KTX' version on the 'WooCommerce' module is pointing to '1.2.0', while on the 'WooCommerce-Wear' module it is pointing to '1.2.1'. This change also updates the 'WooCommerce' version to point to '1.2.1' as well, it being just a batch update that includes a few bugfixes and no more. Release Notes: https://developer.android.com/jetpack/androidx/releases/ preference#1.2.1
FYI: Notice that the 'AndroidX Datastore' versions on the 'WooCommerce' module are pointing to '1.0.0', while on the 'WooCommerce-Wear' module they are pointing to '1.1.0'. This change also updates the 'WooCommerce' versions to point to '1.1.0' as well, it having only additional features on top of '1.0.0' but no breaking changes. Release Notes: https://developer.android.com/jetpack/androidx/releases/ datastore#1.1.0
FYI: The 'androidx-navigation-compose' dependency on the 'WooCommerce' module had no associated version on it, and was most probably taking this version from 'androidx-compose-bom' instead. However, the same dependency on the 'WooCommerce-Wear' module did have one. This change, applies the same version on the 'WooCommerce' module to keep things consistent.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
ParaskP7
force-pushed
the
deps/migrate-to-version-catalogs
branch
from
cc9a507 to
13a9d59
Compare
There was a problem hiding this comment.
Excellent work @ParaskP7 👏, this looks really great, and works well (I tested building both the main app and the Wear one).
I left two minor comments, but none is blocking, so I'm pre-approving.
gradle/libs.versions.toml
Outdated
| androidx-wear-tiles-main = { group = "androidx.wear.tiles", name = "tiles" } | ||
| androidx-wear-tiles-material = { group = "androidx.wear.tiles", name = "tiles-material" } |
There was a problem hiding this comment.
I'd suggest putting an explicit version on this, currently, this dependency is coming as a transitive dependency from the horologist dependencies as well, so they dictate the version 1.3.0, if we remove them, the resolution for the library fails:
So I suggest putting the version 1.3.0 to match the existing one, and make it clearer.
There was a problem hiding this comment.
Great suggestion @hichamboushaba , this is now done: c11e208
build.gradle
Outdated
| alias(libs.plugins.detekt) | ||
| alias(libs.plugins.automattic.measure.builds) | ||
| alias(libs.plugins.dependency.analysis) | ||
| alias(libs.plugins.android.application).apply(false) |
There was a problem hiding this comment.
very minor np, I think that keeping using apply false instead of apply(false) (for example: alias(libs.plugins.android.application) apply false feels more familiar to the previous syntax and looks better 😅, I know it's a small thing, so feel free to ignore.
There was a problem hiding this comment.
Small things matter, this is now done: 970ec5a
…to deps/migrate-to-version-catalogs # Conflicts: # build.gradle
Release Notes: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/releases/tag/2.99.0 The actual update was done as part of 5531327, but after merging this branch with 'trunk', to fully resolved this one conflict, the FluxC update needs to be redone, this time within 'libs.versions.toml'.
As per the c276911 commit, these two 'androidx-wear-tiles' dependencies had no associated version on them. This could be problematic, non deterministic and could cause trouble moving forward. As such, and as per the PR comment suggestion below, this commit adds the most appropriate '1.3.0' version, which is also the one currently being implicitly set transitively by the 'horologist' dependencies themselves. PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 12765#discussion_r1795224428
PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 12765#discussion_r1795251645
|
Thank you so much for the review @hichamboushaba , you rock! 🙇 ❤️ 🚀 Also, apologies for not coming back to you sooner, being on our meetup last week and all. But, I am back now and ready to pick this up with you again! 💯 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Android Lint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #12765 +/- ##
=========================================
Coverage 40.84% 40.84%
Complexity 5744 5744
=========================================
Files 1236 1236
Lines 69687 69687
Branches 9663 9663
=========================================
Hits 28466 28466
Misses 38596 38596
Partials 2625 2625 ☔ View full report in Codecov by Sentry. |
FYI: This is all about the libs.version.toml and the (known) multiple |
|
👋 @hichamboushaba , based on your comment reactions I think we could now proceed and merge this, right? This is just me making it sure before hitting the button? 😊 |
|
Yes, you can go ahead @ParaskP7, I didn't merge it myself in case you needed an input from Wojtek as well. |
|
Great, right, thanks for the confirmation on that @hichamboushaba ! 🙇 @wzieba any additional input from you maybe before me merging this? 🙏 |
…to deps/migrate-to-version-catalogs
|
Awesome, thanks for looking at this and the 👍 @wzieba ! 🙇 🚀 FYI: I just merged the latest changes from |
Project Thread: paaHJt-7cv-p2
This PR migrates all dependencies and plugins in this project to Version Catalogs and under the newly created gradle/libs.versions.toml file.
Description
FYI: I recommend reviewing this PR commit-by-commit just to make sure that every dependency and plugin has been migrated appropriately, just to make sure that there isn't any left-overs that need addressing.
PS: The gradle/libs.versions.toml file, on both, its
versionsandlibrariessections:dash-case) .orgname is added in its prefix to group dependencies together and make them easily identifiable.mainis added in its suffix to make sure that a dependency cannot be a parent of another dependency and a dependency itself. This helps in various corner cases.Extra Focus Required (⚠️ )
Below are some commits you want to put some extra focus on, along with a brief description on each. You can read the full description on the commit itself:
google mlkit-> 9df67a7Google MLKitvsGoogle Play Servicesdependency naming.androidx core-> d967e751.0.0to1.0.1version.androidx preference-> 42b9e111.2.0to1.2.1version.androidx datastore-> 7f69b0c1.0.0to1.1.0version.androidx hilt-> e810a1f1.2.0to1.1.0version.bumptech glide-> 8893f9a@aarfrom version.androidx activity-> 13af208androidx-activity-compose'.androidx compose-> 9e129c0bominstead of1.3.2version.androidx wear-> c276911androidx-wear-tiles.kotlin plugin-> 73e832forg.jetbrains.kotlin.jvmfrom plugins.androidx navigation-> 5f790e8androidx-navigation-compose.On all the above commits I had to make a call, that is, in terms of how to resolve the inconsistency. If any of the changes look suspicious to you, or doesn't seem the right thing to do, please feel free to call me on it and we could revert it (just to keep things as is), or change it to a different resolution (ie. pick a different version).
Testing information
./gradlew buildHealthtask locally and verify that everything is working as expected (CI).RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: