-
-
Notifications
You must be signed in to change notification settings - Fork 521
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
Add Koin for dependency injection #560
Conversation
I've started reviewing but it will take some time since there's a lot of changes. |
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.
LGTM. I noticed a few places where get()
was called multiple times instead of using a property for it but it was all in ugly Java classes so I don't care too much about those.
app/src/main/java/org/jellyfin/androidtv/integration/ChannelManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/jellyfin/androidtv/ui/presentation/ThemeManager.kt
Outdated
Show resolved
Hide resolved
My general approach was to use |
I don't see a problem with that, although I prefer using inject via properties everywhere (but I think you did that already in the Kotlin code) |
Co-authored-by: Niels van Velzen <[email protected]>
Here is an overview of what got changed by this pull request: Issues
======
+ Solved 1
- Added 1
Clones removed
==============
+ app/src/main/java/org/jellyfin/androidtv/ui/playback/CustomPlaybackOverlayFragment.java -1
+ app/src/main/java/org/jellyfin/androidtv/ui/livetv/LiveTvGuideActivity.java -1
See the complete overview on Codacy |
@@ -56,11 +59,11 @@ class NextUpActivity : FragmentActivity() { | |||
} | |||
|
|||
private suspend fun loadItemData(id: String) = withContext(Dispatchers.IO) { | |||
val item = TvApp.getApplication().apiClient.getItem(id) ?: return@withContext null | |||
val item = apiClient.getItem(id) ?: return@withContext null |
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.
Changes
Adds Koin for dependency injection and migrates some global app state to using it instead of referencing TvApp.
Issues
N/A