Convert a bunch of files to kotlin #13026
Conversation
dustdfg
force-pushed
the
kotlin_merged2
branch
2 times, most recently
from
6f8a301 to
2e3cffa
Compare
dustdfg
force-pushed
the
kotlin_merged2
branch
2 times, most recently
from
d7d02f7 to
a549118
Compare
|
No need to convert View related files to Kotlin as they will be deleted later with compose migration. |
theimpulson
left a comment
theimpulson
left a comment
There was a problem hiding this comment.
General feedback that seems to apply to the most of the converted code.
| val result: MutableList<PeertubeInstance> = ArrayList() | ||
| for (instance in array) { | ||
| if (instance is JsonObject) { | ||
| val name = instance.getString("name") | ||
| val url = instance.getString("url") | ||
| result.add(PeertubeInstance(url, name)) | ||
| } | ||
| } | ||
| return result |
There was a problem hiding this comment.
You can probably use .map here to simplify the logic
| } | ||
|
|
||
| @JvmStatic | ||
| fun getTranslatedFilterString(filter: String, c: Context): String { |
There was a problem hiding this comment.
Please rename the parameter to be of a more accurate name like context
|
|
||
| try { | ||
| return NewPipe.getService(serviceName) | ||
| } catch (e: ExtractionException) { |
There was a problem hiding this comment.
Rename unused caught exceptions to _
|
I suggest marking the PR as draft when it is still WIP. Once ready for review, please avoid doing changes unless asked or required to fix or improve existing code in the PR. |
|
There was a problem hiding this comment.
What about this? As stated in commit it also deletes unnecessary code left after another PR. Should I delete this commit from PR (and create separate pr for this) as other view related commits or no?
Seems fair. Might as well convert to Compose in the same PR if you are comfortable with it. |
It would target another branch then |
dustdfg
force-pushed
the
kotlin_merged2
branch
from
a549118 to
301d391
Compare
`onSaveInstanceState` is useless after TeamNewPipe#12995
dustdfg
force-pushed
the
kotlin_merged2
branch
from
301d391 to
906592b
Compare
3 participants
What is it?
Description of the changes in your PR
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