-
Notifications
You must be signed in to change notification settings - Fork 137
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
Resolve androidx.navigation.safeargs.kotlin
in root module
#12337
Conversation
This way, `androidx.navigation.safeargs.kotlin` applied in `WooCommerce` module isn't resolved in isolation, causing creating another classpath. In this particular example, applying this plugin in `WooCommerce` caused `protobuf-java` in version `3.17.2` to be applied. This dependency is already on classpath on main module in version `3.22.3` so resolving `safeargs` in root module classpath will bump `protobuf` to higher version.
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #12337 +/- ##
============================================
- Coverage 40.01% 40.01% -0.01%
+ Complexity 5598 5597 -1
============================================
Files 1219 1219
Lines 69355 69355
Branches 9525 9525
============================================
- Hits 27752 27751 -1
Misses 39073 39073
- Partials 2530 2531 +1 ☔ View full report in Codecov by Sentry. |
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.
👋 @wzieba !
I have reviewed this PR as per the instructions, everything LGTM, great job in figuring this out! 🌟 x 🌟 ^ 🌟
Btw, I tried testing it a bit too, just to make sure I see what you see as well, is that what you see when comparing the builds? 🤔
BEFORE
AFTER
Also, for those 2 security issues below:
- https://github.com/woocommerce/woocommerce-android/security/dependabot/53
- https://github.com/woocommerce/woocommerce-android/security/dependabot/54
Both are suggesting to updade to 1.53.0
or later, but we are anyway at least above 3.17.2
, even before that change, what am I missing... 🤔
The two alerts you linked are about |
Ah, true, thanks for clarifying this for me! 🫣 |
Fixes
Description
This PR moves resolving of
androidx.navigation.safeargs.kotlin
plugin to the root module.This way,
androidx.navigation.safeargs.kotlin
applied inWooCommerce
module isn't resolved in isolation, causing resolution in module-specific classpath.In this particular example, applying this plugin in
WooCommerce
causedprotobuf-java
in version3.17.2
to be applied. This dependency is already on classpath on root module in version3.22.3
so resolvingsafeargs
in root module classpath will bumpprotobuf
to higher version.Testing information
Not necessary: 🟢 from CI checks should be just fine.
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.