-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Upgrade to OkHttp 3. #856
Upgrade to OkHttp 3. #856
Conversation
That looks like an easy upgrade, why is it never that simple? I think this should be a separate integration library called Thanks to their package change it's no longer compatible... so if anyone is stuck with OkHttp 2 due to weird company policies or other reasons they wouldn't be able to use Glide 4 without writing their own integration. |
Build failed with:
|
@@ -0,0 +1,31 @@ | |||
package com.bumptech.glide.integration.okhttp3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the package!
Fixed the build break. Didn't see that when I built locally, though it's probably a bad configuration on my end. Should be good now. The updated PR adds a new OkHttp 3.x integration and deprecates the old 2.x integration. That gives everyone something that'll work. |
private InputStream stream; | ||
private ResponseBody responseBody; | ||
|
||
public OkHttpStreamFetcher(OkHttpClient client, GlideUrl url) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a Call.Factory
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Fixed throughout.
import com.bumptech.glide.load.model.MultiModelLoaderFactory; | ||
|
||
import java.io.InputStream; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build failed with:
integration/okhttp3/OkHttpUrlLoader.java:13: error: Wrong order for 'okhttp3.Call' import.
Try gradlew clean check jar assemble
before commit, it may be a little overkill, but it should check most things if they broke.
Nice job! (Wow, you're really pushing OkHttp 3 :) |
Also upgrade MockWebServer to 3.0.0-RC1, which is versioned with OkHttp. Also deprecate the old OkHttp 2.x integration.
LGTM, thanks all! |
*** Reason for rollback *** Broke [] *** Original change description *** MOE automated commit. ------------- Merge pull request bumptech#854 from luxiaoyu/master fix bug of DiskCacheStrategy.RESOURCE ------------- Merge pull request bumptech#856 from swankjesse/jwilson_0102_okhttp_3 Upgrade to OkHttp 3. ------------- Merge pull request bumptech#874 from TWiStErRob/aar Introduce AAR publish for :library and stop publishing :glide ------------- Merge pull request bumptech#761 from vanniktech/master_add_consumerproguardfiles Add consumerProguardFiles configuration. Fixes bumptech#646 ------------- Mer... *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116167265
*** Reason for rollback *** Rollforward with fixes *** Original change description *** Automated g4 rollback of changelist 116163387. *** Reason for rollback *** Broke [] *** Original change description *** MOE automated commit. ------------- Merge pull request bumptech#854 from luxiaoyu/master fix bug of DiskCacheStrategy.RESOURCE ------------- Merge pull request bumptech#856 from swankjesse/jwilson_0102_okhttp_3 Upgrade to OkHttp 3. ------------- Merge pull request bumptech#874 from TWiStErRob/aar Introduce AAR publish for :library and stop publishing :gl... *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116316494
*** Reason for rollback *** Broke [] *** Original change description *** MOE automated commit. ------------- Merge pull request bumptech#854 from luxiaoyu/master fix bug of DiskCacheStrategy.RESOURCE ------------- Merge pull request bumptech#856 from swankjesse/jwilson_0102_okhttp_3 Upgrade to OkHttp 3. ------------- Merge pull request bumptech#874 from TWiStErRob/aar Introduce AAR publish for :library and stop publishing :glide ------------- Merge pull request bumptech#761 from vanniktech/master_add_consumerproguardfiles Add consumerProguardFiles configuration. Fixes bumptech#646 ------------- Mer... *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116167265
*** Reason for rollback *** Rollforward with fixes *** Original change description *** Automated g4 rollback of changelist 116163387. *** Reason for rollback *** Broke [] *** Original change description *** MOE automated commit. ------------- Merge pull request bumptech#854 from luxiaoyu/master fix bug of DiskCacheStrategy.RESOURCE ------------- Merge pull request bumptech#856 from swankjesse/jwilson_0102_okhttp_3 Upgrade to OkHttp 3. ------------- Merge pull request bumptech#874 from TWiStErRob/aar Introduce AAR publish for :library and stop publishing :gl... *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116316494
No description provided.