Skip to content
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

[Deps] Migrate to Version Catalogs #21262

Merged
merged 93 commits into from
Oct 1, 2024
Merged

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Sep 26, 2024

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 versions and libraries sections:

  • It is alphabetically ordered.
  • It is using the kebal-case naming convention (or else dash-case) .
  • All words are lowercased.
  • Where it makes sense the org name is added in its prefix to group dependencies together and make them easily identifiable.
  • Where needed the word main is 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.

To Test:

  • Smoke test both, the Jetpack and WordPress apps, and verify everything is working as expected.
  • You could also run the ./gradlew buildHealth task locally and verify that everything is working as expected (CI).

Regression Notes

  1. Potential unintended areas of impact

    • Not that I can think of.
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • See To test section above.
  3. What automated tests I added (or what prevented me from doing so)

    • N/A

PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones): N/A

@ParaskP7 ParaskP7 added this to the Future milestone Sep 26, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Sep 26, 2024

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

Build environment changes

The following changes in the build classpath were detected:

list
New Dependencies
com.android.application:com.android.application.gradle.plugin:8.5.1
org.jetbrains.kotlin.android:org.jetbrains.kotlin.android.gradle.plugin:1.9.24
org.jetbrains.kotlin.jvm:org.jetbrains.kotlin.jvm.gradle.plugin:1.9.24
org.jetbrains.kotlin.plugin.allopen:org.jetbrains.kotlin.plugin.allopen.gradle.plugin:1.9.24
org.jetbrains.kotlin.plugin.serialization:org.jetbrains.kotlin.plugin.serialization.gradle.plugin:1.9.24
org.jetbrains.kotlin:kotlin-allopen:1.9.24
org.jetbrains.kotlin:kotlin-serialization:1.9.24
tree
++--- androidx.navigation.safeargs.kotlin:androidx.navigation.safeargs.kotlin.gradle.plugin:2.7.7
+|    \--- androidx.navigation:navigation-safe-args-gradle-plugin:2.7.7
+|         \--- org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.0 -> 1.9.24
+|              \--- org.jetbrains.kotlin:kotlin-gradle-plugins-bom:1.9.24
+|                   +--- org.jetbrains.kotlin:kotlin-allopen:1.9.24 (c)
+|                   \--- org.jetbrains.kotlin:kotlin-serialization:1.9.24 (c)
++--- com.android.application:com.android.application.gradle.plugin:8.5.1
+|    \--- com.android.tools.build:gradle:8.5.1 (*)
++--- org.jetbrains.kotlin.plugin.allopen:org.jetbrains.kotlin.plugin.allopen.gradle.plugin:1.9.24
+|    \--- org.jetbrains.kotlin:kotlin-allopen:1.9.24
+|         +--- org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.9.24 (*)
+|         +--- org.jetbrains.kotlin:kotlin-gradle-plugins-bom:1.9.24 (*)
+|         \--- org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.9.24 (*)
++--- org.jetbrains.kotlin.android:org.jetbrains.kotlin.android.gradle.plugin:1.9.24
+|    \--- org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.24 (*)
++--- org.jetbrains.kotlin.jvm:org.jetbrains.kotlin.jvm.gradle.plugin:1.9.24
+|    \--- org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.24 (*)
+\--- org.jetbrains.kotlin.plugin.serialization:org.jetbrains.kotlin.plugin.serialization.gradle.plugin:1.9.24
+     \--- org.jetbrains.kotlin:kotlin-serialization:1.9.24
+          +--- org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.9.24 (*)
+          \--- org.jetbrains.kotlin:kotlin-gradle-plugins-bom:1.9.24 (*)

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 26, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21262-c8240a7
Commitc8240a7
Direct Downloadjetpack-prototype-build-pr21262-c8240a7.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 26, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21262-c8240a7
Commitc8240a7
Direct Downloadwordpress-prototype-build-pr21262-c8240a7.apk
Note: Google Login is not supported on these builds.

This configuration was added as part of this
b2bf8b1 initial module setup commit,
but never had any effect. As such, it is removed from the list of
dependencies to make cleanup the dependency block from unnecessary
configuration, which also needs not be migrated to version catalogs.
@ParaskP7 ParaskP7 force-pushed the deps/migrate-to-version-catalogs branch from a1fa658 to d7fa850 Compare September 27, 2024 08:18
@ParaskP7 ParaskP7 marked this pull request as ready for review September 27, 2024 09:51
@nbradbury nbradbury self-assigned this Sep 30, 2024
nbradbury

This comment was marked as resolved.

@@ -43,8 +43,6 @@ repositories {
}

dependencies {
implementation fileTree(dir: 'libs', include: ['*.jar'])
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -24,10 +24,6 @@ ext {
ext {
// main
androidxComposeNavigationVersion = '2.7.7'
androidxGridlayoutVersion = '1.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

@ParaskP7 I'm just curious why these three libraries weren't flagged as unused by buildHealth when I worked on #21238?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👋 @nbradbury !

These variables you see there are just versions that could be potentially used (were used in the past) on a dependency/library. When there is no dependency associated with those, there is nothing buildHealth or :dependabot: can do identify those "unused" version related variables to report them as such.

Does that make sense? 🙏

Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

@ParaskP7 Fantastic work on this! It took me a bit but I went commit by commit and didn't find any issues. I'll approve but perhaps @wzieba wants to take a look before merging? :shipit:

Copy link
Contributor

@wzieba wzieba left a comment

Choose a reason for hiding this comment

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

Looks good. I'm especially glad that the runtime dependencies are not change by this PR, which proves a thoughtful migration. 👏

About build classpath changes - they're expected, as we now declare (but not apply) more plugins in the root project. I'm okay with this and I think this is even better as some plugins authors expect us to do this, e.g. AGP (docs).

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Oct 1, 2024

Awesome, thank you both @nbradbury and @wzieba for the review, to version catalogs and beyond! 🎉 ❤️ 🚀

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Oct 1, 2024

FYI: I just noticed yesterday a missing main suffix on the androidx-constraintlayout version, so I just added it in this c8240a7 commit and when CI finishes I'll proceed with merging this PR.

Copy link

sonarcloud bot commented Oct 1, 2024

@ParaskP7 ParaskP7 merged commit 8152531 into trunk Oct 1, 2024
21 checks passed
@ParaskP7 ParaskP7 deleted the deps/migrate-to-version-catalogs branch October 1, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants