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

Support SDWebImage both v3 and v4 #804

Merged
merged 5 commits into from
Dec 8, 2017
Merged

Conversation

Lax
Copy link
Contributor

@Lax Lax commented Dec 7, 2017

  • Update table cell remote_image according to SDWebImage API changes.
    See "SDWebImage 4.0 Migration Guide"
  • Update travis.yml because travis-ci dropped xcode-7.2 support

@@ -63,10 +63,10 @@ def set_remote_image
self.imageView.image = remote_placeholder

if sd_web_image?
@remote_image_operation = SDWebImageManager.sharedManager.downloadWithURL(data_cell[:remote_image][:url].to_url,
options:SDWebImageRefreshCached,
@remote_image_operation = SDWebImageManager.sharedManager.loadImageWithURL(data_cell[:remote_image][:url].to_url,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change will break for anyone who is still using v3. You could change this to support both v3 and v4 by checking to see if the method exists before calling it, like this:

if SDWebImageManager.sharedManager.respond_to?('downloadWithURL:options:progress:completed')
  # old v3 code here
else
  # new v4 code here
end

@andrewhavens
Copy link
Collaborator

Thanks for your work on this @Lax! If you could make that simple change to support both v3 and v4 that I mentioned above, I will merge it.

@Lax Lax changed the title SDWebImage 3.x -> 4.x Migration Support SDWebImage both v3 and v4 Dec 8, 2017
@andrewhavens andrewhavens merged commit 88c654a into jamonholmgren:master Dec 8, 2017
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