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

[#46, #90] Upgrade Okhttp library to 3.6.0 and enable AOT #93

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathannaguin
Copy link

  • Upgrade OkHttp to 3.6.0.
  • Adding AOT for every project.
  • Dropping support for retrofit, new version of retrofit (2.x) has renamed RestTemplate to Retrofit and now its Builder is final. However the builder supports a OkHttpClient as an option, so it might be possible to use comsat-okhttp client as a parameter to the builder.

Jonathan Nunez Aguin added 2 commits April 14, 2017 17:52
Dropping support for retrofit, new version of retrofit (2.x) has renamed RestTemplate to Retrofit and now its Builder is final. However the builder supports a OkHttpClient as an option, so it might be possible to use comsat-okhttp client as a parameter to the builder.
@jonathannaguin
Copy link
Author

@pron could anyone review this PR please? It seems some of the tests are failing but don't think it's related to my changes though

@pron
Copy link
Contributor

pron commented Apr 27, 2017

This is a very big PR. Thank you for taking the time to do it. But it does do quite a few separate things, and should probably be broken down.

  1. Can you explain what you mean by "adding AOT for every project"?

  2. Let's separate the removal of Retrofit because a new version is problematic into a separate issue.

@jonathannaguin
Copy link
Author

  1. In Gradle, I included the compile instrumentation for Fiber in every subproject.
  2. Do you want another PR for removing Retrofit? If I rolled back the removal then this PR will not compile at all, as the Retrofit module will stop working at compile time.

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.

2 participants