Skip to content

Convert part of Tab logic to kotlin#12939

Closed
ghost wants to merge 1 commit intorefactorfrom
unknown repository
Closed

Convert part of Tab logic to kotlin#12939
ghost wants to merge 1 commit intorefactorfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Dec 25, 2025

What is it?

  • Bugfix (user facing)
  • Feature (user facing) ⚠️ Your PR must target the refactor branch
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Here I converted settings/tabs/Tab.kt from java to kotlin

Tbh I am not even fully sure if it works (I tested but ¯\_(ツ)_/¯). And pr as a whole a bit messy because I know kotlin very bad and for example some fields are declared as nullable to just be 100% set to not null. Thanks to the fact it is a kotlin some things become cleaner but as I said some become less cleaner. So why am I sending this PR? I can't do better for now so sorry for sending this crap... Maybe doesn't look at it!

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

@github-actions github-actions Bot added the size/giant PRs with more than 750 changed lines label Dec 25, 2025
import org.schabi.newpipe.util.ServiceHelper
import java.util.Objects

abstract class Tab {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to make it a data class and stop overriding hashcode and equals methods

Copy link
Copy Markdown
Author

@ghost ghost Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to make Tab a data class? It would mean I can't make it abstract. I am almost sure it is possible to make inner classes to be data classes but not top class.

I also though about it and about composition but the more I edit the less I am sure code still is doing the same...

@TobiGr
Copy link
Copy Markdown
Contributor

TobiGr commented Dec 29, 2025

Tbh I am not even fully sure if it works (I tested but ¯_(ツ)_/¯). And pr as a whole a bit messy because I know kotlin very bad and for example some fields are declared as nullable to just be 100% set to not null. Thanks to the fact it is a kotlin some things become cleaner but as I said some become less cleaner. So why am I sending this PR? I can't do better for now so sorry for sending this crap... Maybe doesn't look at it!

Thank you for your interesting in improving NewPipe and bringing the refactoring process forward! However, the refactoring process is there to thoroughly review the old code, identify bad architecture and parts in general, and then to improve these. I really appreciate your interest in helping with the process but as you already mentioned that your knowledge of kotlin is not that good, it might be better for you to take a look at different things. If you have a good Java knowledge you might want to fix bugs in the dev version or work on the extractor which are both mostly written in Java. We'll close those PRs which do not meet the goals of the refactoring process.

@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 29, 2025

Tbh I am not even fully sure if it works (I tested but ¯_(ツ)_/¯). And pr as a whole a bit messy because I know kotlin very bad and for example some fields are declared as nullable to just be 100% set to not null. Thanks to the fact it is a kotlin some things become cleaner but as I said some become less cleaner. So why am I sending this PR? I can't do better for now so sorry for sending this crap... Maybe doesn't look at it!

Thank you for your interesting in improving NewPipe and bringing the refactoring process forward! However, the refactoring process is there to thoroughly review the old code, identify bad architecture and parts in general, and then to improve these. I really appreciate your interest in helping with the process but as you already mentioned that your knowledge of kotlin is not that good, it might be better for you to take a look at different things. If you have a good Java knowledge you might want to fix bugs in the dev version or work on the extractor which are both mostly written in Java.

I am bad at everything. So bad that now I know kotlin better than java...

We'll close those PRs which do not meet the goals of the refactoring process.

I understand. (I thought, I've already closed this pr)

@ghost ghost closed this Dec 29, 2025
@ghost ghost deleted the kotlin_refactor branch January 7, 2026 18:53
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/giant PRs with more than 750 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants