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

Removing deprecated charset=utf-8 from Content-Type header #2883

Merged

Conversation

vikrama
Copy link
Contributor

@vikrama vikrama commented Jan 18, 2021

Addresses #2882 . Keeping the string in api module as per Martin's suggestion.

@apollo-cla
Copy link

@vikrama: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@martinbonnin martinbonnin merged commit 49d4768 into apollographql:main Jan 20, 2021
@martinbonnin
Copy link
Contributor

@vikrama Thanks for the contribution 🤗

I moved everything to the api.internal package, I don't think this needs to be seen from consumers. We can always add it later if needed.

@vikrama
Copy link
Contributor Author

vikrama commented Jan 21, 2021

Thx @martinbonnin for the review.

Not urgent, but trying to understand why my last build failed. Seems unrelated to my changes. Will help me be better informed while sending PRs in the future.

@martinbonnin
Copy link
Contributor

martinbonnin commented Jan 21, 2021

trying to understand why my last build failed. Seems unrelated to my changes. Will help me be better informed while sending PRs in the future.

Are you refering about this build failure? https://github.com/apollographql/apollo-android/runs/1724495722

This is because we track changes to the public API with metalava https://android.googlesource.com/platform/tools/metalava/

This we we're notified any time the public API breaks. For this to work, we also have to track we new additions are made like is the case here.

To fix the build, you can run ./gradlew metalavaGenerateSignature and commit the updates api.txt files

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.

None yet

3 participants