Skip to content
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 ExceptionUtils methods as extension functions. #4947

Merged

Conversation

Isira-Seneviratne
Copy link
Member

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Rewrite the ExceptionUtils methods as extension functions for better discoverability in Kotlin.

APK testing

app-debug.zip

Due diligence

@Isira-Seneviratne Isira-Seneviratne force-pushed the Convert_ExceptionUtils_to_extensions branch 2 times, most recently from f8c5d42 to 5959da1 Compare November 29, 2020 02:15
XiangRongLin
XiangRongLin previously approved these changes Dec 12, 2020
Copy link
Collaborator

@XiangRongLin XiangRongLin left a comment

Choose a reason for hiding this comment

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

From code standpoint it looks fine to me. I did not try the apk.

@file:JvmName("ExceptionUtils") to keep the java world happy is nice and in the IDE it does get correctly indexed/linked.

Only thing i have is that the test class for Throwable is called ThrowableExtensionsTest, but that should be fine, if NewPipe wants to stick with this style.

Stypox
Stypox previously approved these changes Dec 15, 2020
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

I tested and it works. Thank you for all of the improvements :-D

but that should be fine, if NewPipe wants to stick with this style.

Yeah, I think this is ok.

@Stypox
Copy link
Member

Stypox commented Dec 15, 2020

Oh, but compilation fails, could you fix this?

@Isira-Seneviratne
Copy link
Member Author

Oh, but compilation fails, could you fix this?

I did.

@Isira-Seneviratne Isira-Seneviratne marked this pull request as draft December 15, 2020 10:40
@Isira-Seneviratne Isira-Seneviratne marked this pull request as ready for review December 15, 2020 10:54
@Isira-Seneviratne Isira-Seneviratne force-pushed the Convert_ExceptionUtils_to_extensions branch 2 times, most recently from a290684 to f2bb8b1 Compare December 19, 2020 23:10
@Isira-Seneviratne Isira-Seneviratne force-pushed the Convert_ExceptionUtils_to_extensions branch from f2bb8b1 to 8b7c597 Compare January 2, 2021 03:52
@Isira-Seneviratne Isira-Seneviratne force-pushed the Convert_ExceptionUtils_to_extensions branch from 8b7c597 to 50dcf30 Compare January 11, 2021 11:20
Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Looks good to me

@Stypox Can we merge this?

@Stypox Stypox merged commit 9ee7740 into TeamNewPipe:dev Jan 14, 2021
@Isira-Seneviratne Isira-Seneviratne deleted the Convert_ExceptionUtils_to_extensions branch January 15, 2021 01:46
This was referenced Jan 18, 2021
tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 20, 2021
…nvert_ExceptionUtils_to_extensions

Rewrite ExceptionUtils methods as extension functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants