-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Migrate newpipe.error to Kotlin #10922
base: dev
Are you sure you want to change the base?
Conversation
Only error was a nullable check in AcraReportSender, which I fixed by replacing null with an empty array. I tested the error report by producing a network error and opening the report, creating an email and accepting the EULA. Warnings will be fixed in the next commit.
We will init them at the beginning of the view lifecycle, so they are definitly not null.
Quality Gate passedIssues Measures |
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.
Formatting seems off (remember to set up ktlint)
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.
This class seems unused
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.
I think something went wrong with your review? The code you're talking about isn't showing up. Just the file names.
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.
Well yea, I wanted to point to the file name. AcraReportSender
seems to be unused, I think the file is safe to remove instead of converting to Kotlin.
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.
Oh. I didn't know you could leave a review for an entire file.
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.
There are merge conflicts with this enum. Consider pulling the latest changes from dev and re-generate the .kt file
An experiment in how much effort it is to migrate classes to kotlin.
It took me about 30–60 minutes (minus getting to know some kotlin features) to migrate these, try the result and fix all warnings that popped up.
What is it?
Due diligence