-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Rewrite import and export subscriptions functionality using coroutines #11759
base: refactor
Are you sure you want to change the base?
Rewrite import and export subscriptions functionality using coroutines #11759
Conversation
|
try { | ||
@OptIn(ExperimentalSerializationApi::class) | ||
return json.decodeFromStream<SubscriptionData>(`in`).subscriptions | ||
} catch (e: Throwable) { | ||
throw InvalidSourceException("Couldn't parse json", e) | ||
} | ||
} |
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.
From a cursory look, this seems to be a lot stricter than the previous json parser.
e.g. the serviceId
would default to 0
, and items in the list would simply be skipped if they don’t conform to the schema.
Do we want to change that behavior?
app/src/main/java/org/schabi/newpipe/local/subscription/ImportConfirmationDialog.java
Outdated
Show resolved
Hide resolved
e99c0bc
to
dbd11a6
Compare
I rebased on current Unfortunately, there is no |
app/src/main/java/org/schabi/newpipe/local/subscription/workers/SubscriptionImportWorker.kt
Outdated
Show resolved
Hide resolved
Once the missing error handling is added on import & the question about how leniently to parse the json is resolved, I’d say LGTM! |
# Conflicts: # gradle/libs.versions.toml
144d7f1
to
abaf16e
Compare
|
What is it?
Description of the changes in your PR
CoroutineWorker
for better performance and readability of the code.Before/After Screenshots/Screen Record
Export
Screen_recording_20241129_071419.webm
Android 6.0
Screen_recording_20241129_064527.webm
Android 6.0
Screen_recording_20241129_064829.mp4
Android 15
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence