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

Cleanup of YoutubeStreamExtractor and some related classes #782

Merged
merged 1 commit into from
May 1, 2022

Conversation

litetex
Copy link
Member

@litetex litetex commented Jan 30, 2022

Code cleanup - Summary what I did:

  • Fixed obvious sonar(lint) warnings
  • Abstracted some code (get*Streams)
  • Used some new lines to make code better readable
  • Chopped down brace-jungle in some methods
  • Use StandardCharset (Java 8 4tw)
  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe. - No but tests work fine
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API. - No changes required for NewPipe

@litetex litetex added the youtube service, https://www.youtube.com/ label Jan 30, 2022
@litetex litetex marked this pull request as ready for review February 13, 2022 15:27
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.

Looks good to me, except for the internal discussion

Btw, maybe Utils.UTF_8 should be deprecated in a separate PR, if at the end we decide to do it

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.

Almost ready

@litetex litetex force-pushed the cleanup-yt-stream-extractor branch 2 times, most recently from e1f8926 to 97e3e0f Compare March 19, 2022 16:21
@litetex litetex requested a review from Stypox March 26, 2022 16:26
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.

Thank you! This needs rebasing, but once you rebase feel free to merge directly

continue;
}

try {
Copy link
Member

Choose a reason for hiding this comment

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

I guess the only reason you didn't convert this into streams is because there is exception handling inside it ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, Parsing exception is not unchecked

* Fixed obvious sonar(lint) warnings
* Abstracted some code (get*Streams)
* Used some new lines to make code better readable
* Chopped down brace-jungle in some methods
* Use StandardCharset (Java 8 4tw)
@litetex litetex force-pushed the cleanup-yt-stream-extractor branch from 97e3e0f to fe30eb4 Compare May 1, 2022 14:39
@litetex litetex merged commit 5db4d1f into TeamNewPipe:dev May 1, 2022
@litetex litetex deleted the cleanup-yt-stream-extractor branch May 1, 2022 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
youtube service, https://www.youtube.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants