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

Use charset string to support Android 16 #2111

Merged
merged 2 commits into from
Jun 20, 2022
Merged

Conversation

adinauer
Copy link
Member

📜 Description

Use charset string directly as StandardCharsets is not available on earlier Android versions

💡 Motivation and Context

Field requires API level 19 (current min is 16): java.nio.charset.StandardCharsets#UTF_8

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

@bruno-garcia bruno-garcia changed the title Use charset string directly as StandardCharsets is not available on e… Use charset string to support Android 16 Jun 20, 2022
@@ -16,7 +15,7 @@
@ApiStatus.Experimental
public final class Baggage {

static final @NotNull String CHARSET = StandardCharsets.UTF_8.toString();
static final @NotNull String CHARSET = "UTF-8";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static final @NotNull String CHARSET = "UTF-8";
private static final Charset UTF_8 = Charset.forName("UTF-8");

Just because that's how we use it everywhere, so we keep the pattern.

Copy link
Member Author

@adinauer adinauer Jun 20, 2022

Choose a reason for hiding this comment

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

Can't because of

Usage of API documented as @ since 10+

when using your suggestion it uses another method here:
URLDecoder.decode(value, UTF_8)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine, our SDK is 14+ (10 years old)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is about JDK version, not Android version.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

left a comment but otherwise LGTM

@adinauer adinauer merged commit 6f77e2e into main Jun 20, 2022
@adinauer adinauer deleted the feat/fix-charset-for-baggage branch June 20, 2022 13:44
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.

3 participants