Skip to content

[Dependency Updates] Update androidxArchCoreVersion to 2.1.0#17642

Merged
irfano merged 2 commits intodeps/main-batch-androidx-corefrom
deps/update-androidx-arch-core-to-2.1.0
Dec 9, 2022
Merged

[Dependency Updates] Update androidxArchCoreVersion to 2.1.0#17642
irfano merged 2 commits intodeps/main-batch-androidx-corefrom
deps/update-androidx-arch-core-to-2.1.0

Conversation

@ParaskP7
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 commented Dec 8, 2022 •
edited
Loading

Parent #17561
Batch Branch: deps/main-batch-androidx-core

This PR updates androidxArchCoreVersion to 2.1.0.

Also, as part of this update the below transitive dependencies were added on the WordPress module (8884a00):

  • androidx.arch.core:core-common
  • androidx.arch.core:core-runtime

PS: @irfano I added you as the main reviewer, that is, in addition to the @wordpress-mobile/apps-infrastructure team itself, but randomly, since I just wanted someone from the WordPress team to be aware of and sign-off on that change for WPAndroid.

To test:

  1. There is nothing much to test here.
  2. Verifying that all the CI checks are successful should be enough.
  3. However, if you want to be thorough about reviewing this change, you could:
    1. See the dependency tree diff result and verify correctness.
    2. Quickly smoke test both, the WordPress and Jetpack apps, and see if they both work as expected.

Note the fact that:

  • This androidxArchCoreVersion version is only related to testing (see androidx.arch.core:core-testing). See androidx.arch.core related imports.
  • Transitively, this androidxArchCoreVersion dependency was already pointing to 2.1.0 anyway.

Regression Notes

  1. Potential unintended areas of impact

Some of the transitive dependencies added might be causing some kind of misbehaviour.

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

See To test section above.

  1. 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.

@ParaskP7
Deps: Add arch core related missing transitively used dependencies
8884a00 
It is generally recommended that transitively used dependencies should
be declared directly.
@ParaskP7
Deps: Update androidx arch core to 2.1.0
0c504ad 
Release Notes: https://developer.android.com/jetpack/androidx/releases/
arch-core#2.1.0
@ParaskP7 ParaskP7 added this to the Future milestone Dec 8, 2022
@ParaskP7 ParaskP7 requested review from a team and irfano December 8, 2022 10:34
@ParaskP7 ParaskP7 self-assigned this Dec 8, 2022
@ParaskP7 ParaskP7 changed the base branch from trunk to deps/main-batch-androidx-core December 8, 2022 10:34
@ParaskP7 ParaskP7 changed the title Update androidxArchCoreVersion to 2.1.0 [Dependency Updates] Update androidxArchCoreVersion to 2.1.0 Dec 8, 2022
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Dec 8, 2022

Found 1 violations:

The PR caused the following dependency changes:

++--- androidx.arch.core:core-common:2.1.0 (*)
+\--- androidx.arch.core:core-runtime:2.1.0 (*)

Please review and act accordingly

@ParaskP7 ParaskP7 mentioned this pull request Dec 8, 2022
Closed
6 tasks
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Dec 8, 2022

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17642-0c504ad.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commit0c504ad
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Dec 8, 2022

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17642-0c504ad.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commit0c504ad
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

irfano
irfano reviewed Dec 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@irfano irfano left a comment

Choose a reason for hiding this comment

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

@ParaskP7, I tested the basic features on both WP and JP, and they work as expected.
I just added a question on the code.

WordPress/build.gradle
Comment on lines +373 to +374
implementation "androidx.arch.core:core-common:$androidxArchCoreVersion"
implementation "androidx.arch.core:core-runtime:$androidxArchCoreVersion"
Copy link
Copy Markdown
Member

@irfano irfano Dec 9, 2022

Choose a reason for hiding this comment

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

Why did we add these? I removed them, and the application, unit tests, and android tests still worked.

Copy link
Copy Markdown
Contributor Author

@ParaskP7 ParaskP7 Dec 9, 2022

Choose a reason for hiding this comment

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

👋 @irfano and thanks for the question! 🌟

As per this commit and its description, those are AndroidX Arch Core related missing transitively used dependencies. It is generally recommended that transitively used dependencies should be declared directly. I can expand on that if you like. 🤔

FYI: Note the fact that I got this advise from the Dependency Analysis tool that I am currently using while working with dependency updates, just to make our dependency management a bit more manageable.

PS: You can also check more info about that on this P2 post that I wrote a while back (paqN3M-Iq-p2).

Copy link
Copy Markdown
Member

@irfano irfano Dec 9, 2022

Choose a reason for hiding this comment

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

Thank you for the detailed explanation. I guess I understood the reason.
LGTM! Approving.

@ParaskP7
Copy link
Copy Markdown
Contributor Author

ParaskP7 commented Dec 9, 2022

Thank you a lot for reviewing and testing this PR @irfano , you are awesome! 🙇 ❤️ 🚀

@irfano irfano merged commit ce51f00 into deps/main-batch-androidx-core Dec 9, 2022
@irfano irfano deleted the deps/update-androidx-arch-core-to-2.1.0 branch December 9, 2022 12:44
@ParaskP7 ParaskP7 mentioned this pull request Jan 11, 2023
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@irfano irfano irfano approved these changes

Assignees

@ParaskP7 ParaskP7

Labels

Tech Debt

Projects

None yet

Milestone

Future

Development

Successfully merging this pull request may close these issues.

3 participants

@ParaskP7 @wpmobilebot @irfano