-
Notifications
You must be signed in to change notification settings - Fork 517
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
Fix #3605: Introduce Platform Parameter WorkManager classes #3606
Conversation
…esting/platformparameter BUILD.bazel
…and Gae models also adding test cases in Service test and documentation over Gae models
…eModels and used a Map<String,Any> instead
…_DEPS of testing module
…ameter Service Impl in Network Module
Unassigning @vinitamurthi since they have already approved the PR. |
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.
Thanks @Arjupta. Followed up on all recent changes/comments. PTAL.
...pia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializer.kt
Outdated
Show resolved
Hide resolved
utility/src/main/java/org/oppia/android/util/platformparameter/PlatformParameterConstants.kt
Outdated
Show resolved
Hide resolved
...android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializerTest.kt
Outdated
Show resolved
Hide resolved
.../src/test/java/org/oppia/android/domain/platformparameter/PlatformParameterControllerTest.kt
Outdated
Show resolved
Hide resolved
...main/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorker.kt
Outdated
Show resolved
Hide resolved
...android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializerTest.kt
Outdated
Show resolved
Hide resolved
.../java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkerTest.kt
Show resolved
Hide resolved
...main/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorker.kt
Outdated
Show resolved
Hide resolved
...main/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorker.kt
Outdated
Show resolved
Hide resolved
...pia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializer.kt
Outdated
Show resolved
Hide resolved
Also @Arjupta I suggest syncing with the latest develop changes & updating this PR accordingly. |
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.
Thanks @Arjupta. Just a couple of new comments, and one existing thread that I think hasn't yet been resolved. Other than these last few comments the PR LGTM.
PTAL.
...pia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializer.kt
Outdated
Show resolved
Hide resolved
...pia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializer.kt
Show resolved
Hide resolved
...main/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorker.kt
Outdated
Show resolved
Hide resolved
...pia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorkManagerInitializer.kt
Outdated
Show resolved
Hide resolved
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.
Thanks @Arjupta. This LGTM!
...main/java/org/oppia/android/domain/platformparameter/syncup/PlatformParameterSyncUpWorker.kt
Outdated
Show resolved
Hide resolved
Given other threads seem resolved & everyone has approved, merging this. |
Explanation
Fixes #3605
Introduced a Lightweight Syncing mechanism for PlatformParameter with the help of Work Manager. This PR was stacked over #3489
Checklist