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.

Sign up for GitHub

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

Jump to bottom

Use StandardCharsets.UTF_8. #880

Merged
merged 5 commits into from
Nov 12, 2022
Merged

Use StandardCharsets.UTF_8. #880

merged 5 commits into from
Nov 12, 2022

Conversation

Isira-Seneviratne
Copy link
Member

Stypox
Stypox previously requested changes Aug 4, 2022
View reviewed changes
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.

Mmmh, maybe utility methods encodeUrlUTF8() and decodeUrlUTF8() should be created in Utils.java, since I see URLEncoder.encode(url, UTF_8) being used quite often.

@Stypox
Copy link
Member

Stypox commented Aug 4, 2022

Reminder for myself: open a PR that separates utility methods in more classes, once this is merged

@Isira-Seneviratne Isira-Seneviratne force-pushed the Use_StandardCharsets branch 4 times, most recently from e2e24ce to 858c40a Compare August 12, 2022 23:40
@Isira-Seneviratne Isira-Seneviratne mentioned this pull request Aug 13, 2022
Merged
5 tasks
@ag2s20150909
Copy link

StandardCharsets.UTF_8 is

@Isira-Seneviratne
Copy link
Member Author

Isira-Seneviratne commented Aug 13, 2022 •
edited
Loading

StandardCharsets.UTF_8 is

Support for desugaring StandardCharsets for API levels below 19 is being added in a new version of the desugar library: google/desugar_jdk_libs@423a0d9

@Isira-Seneviratne Isira-Seneviratne force-pushed the Use_StandardCharsets branch 3 times, most recently from c44e242 to a40c66b Compare August 26, 2022 02:44
@AudricV AudricV requested a review from Stypox September 8, 2022 15:51
TobiGr
TobiGr approved these changes Oct 10, 2022
View reviewed changes
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.

Changes look good to me.
Please rebase so we can see whether any new tests fail.

@Isira-Seneviratne Isira-Seneviratne force-pushed the Use_StandardCharsets branch 4 times, most recently from 51f1429 to 9e59a77 Compare November 7, 2022 02:21
TobiGr
TobiGr approved these changes Nov 12, 2022
View reviewed changes
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.

I created a separate PR for the failing tests (#937). I'll merge this anyway

@TobiGr TobiGr merged commit 43d1c1f into TeamNewPipe:dev Nov 12, 2022
@Isira-Seneviratne Isira-Seneviratne deleted the Use_StandardCharsets branch November 12, 2022 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TobiGr TobiGr TobiGr approved these changes

@FireMasterK FireMasterK FireMasterK approved these changes

@Stypox Stypox Awaiting requested review from Stypox

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Isira-Seneviratne @Stypox @ag2s20150909 @TobiGr @FireMasterK