Kitsu added as sync provider #2440
Conversation
…dded to BackupUtils.kt, Kitsu panel now does pagination fine
…ate, and use a unique API call to get the library item and map it to the anime
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Very good code, I will leave a more detailed review after I have tested it.
|
|
||
| override suspend fun load(auth : AuthData?, id: String): SyncResult? { | ||
| val auth = auth?.token?.accessToken ?: return null | ||
| id.toIntOrNull() ?: return null |
There was a problem hiding this comment.
Did you mean to write val id = id.toIntOrNull() ?: return null because this is confusing to read even if its only purpose is to return null on non-int ids.
There was a problem hiding this comment.
I write it like that to return null if the id failed on converting to int. Now that I look at it, it is confusing hahaha. I can change it to
if (id.toIntOrNull() == null) {
return null
}or not doing the comprobation. In fact I think this is the best option because Kitsu uses numbers inside a string so in a future they might introduce other strings that are not numeric values
There was a problem hiding this comment.
Yes, do that as it becomes clear what exactly you are trying to do. Otherwise one might think that you can refactor the code by removing your current statement.
| val fullList = mutableListOf<KitsuNode>() | ||
| val listMutex = Mutex(false) | ||
|
|
||
| coroutineScope { |
There was a problem hiding this comment.
Why? From the looks of it a simple while loop should work fine?
There was a problem hiding this comment.
Yeah, right now it can be a normal loop, the initial reason of that was because I misunderstood the way the Kitsu api worked and to achieve a decent result in time I need a lot of concurrency.
I will modify to use a simple loop
| this.addSimklId(SimklSyncServices.Mal, id.toString()) | ||
| } | ||
|
|
||
| fun LoadResponse.addKitsuId(id: Int?) { |
There was a problem hiding this comment.
Add a prerelease annotation.
There was a problem hiding this comment.
Do you mean a comment in top saying "Prerelease" ?
There was a problem hiding this comment.
No, we use the annotation @Prerelease
…mple while loop, simpler and more complete in data load function
Review changes for pull request on main repo
|
In the way of doing the changes requested I have also corrected the updateStatus functionality when None is selected in the status to delete that anime from the library |
2 participants
This pull request is trying to resolve the issue #2031.
In the issue I posted screenshots on what it looks like implemented
Implemented functionality: