Hilt migration: Add AppInitializer#16055
Conversation
WellSqlInitializer is added for debug and release variants. It'll be used in AppInitializer.
AppInitializer is created, and all initializing stuff is moved to AppInitializer from WordPress.
Generated by 🚫 dangerJS |
hichamboushaba
left a comment
There was a problem hiding this comment.
Sorry for the late review, the code looks good to me, nice work, and thanks @irfano for working on this 👍.
I didn't approve just to give @renanferrari and @ravishanker a chance to review too, and test the app before merging.
| androidx.work.Configuration config = | ||
| (new androidx.work.Configuration.Builder()).setWorkerFactory(mWordPressWorkerFactory).build(); | ||
| WorkManager.initialize(this, config); | ||
| return AppInitializer.Companion.getBitmapCache(); |
There was a problem hiding this comment.
np, since we have the annotation @JvmStatic on the function, we can access it without passing by the companion object.
There was a problem hiding this comment.
Good catch! 👍🏻
I'm removing @JvmStatic annotation from functions instead of removing Companion. Because we won't need @JvmStatic when we convert WordPress to Kotlin.
# Conflicts: # WordPress/src/main/java/org/wordpress/android/WordPress.java
|
I changed the base from |
ravishanker
left a comment
There was a problem hiding this comment.
It's a good plan to collate all this effort into a separate branch before merging it into trunk. Make sure to pull in trunk from time to time to avoid having to resolve conflicts.
LGTM 👍
This PR is preparation for Hilt migration. Hilt migration is implemented in #15653 but it's a big PR and a bit outdated. This PR is a small part of #15653.
Basically, this PR includes only adding
AppInitializerwithout adding Hilt dependency.Why will we need AppInitializer?
After Hilt is added, the UI test structure will change completely. Hilt will use a test application different from real application. The main reason is that Hilt will inject components at different times in test and real applications. By using
AppInitializerwe'll be able to control initializing by just deciding when to callAppInitilizer'sinit()function.Even though we don't add Hilt in this PR, it's still better to handle initialization in a separate class rather than WordPress class.
To test:
There shouldn't be any user-facing change. It can be smoke tested.
Regression Notes
Potential unintended areas of impact
Login, logout functions, notifications. Since the logic from the core is moved to another class, most of the features can be affected.
What I did to test those areas of impact (or what existing automated tests I relied on)
I smoke tested the app, it's working as expected.
What automated tests I added (or what prevented me from doing so)
No tests are added since no new feature is added.
PR submission checklist:
RELEASE-NOTES.txtif necessary.