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

Pass imageURL in completedBlock #770

Merged
merged 13 commits into from
Jun 19, 2014
Merged

Conversation

bpoplauschi
Copy link
Member

Redone #759 (contained some changes that were already merged, did not replace and deprecate all methods, method names could be better, wanted to redo the docs, indentation was inconsistent):

  • deprecated block type SDWebImageCompletedWithFinishedBlock, replaced with SDWebImageCompletionWithFinishedBlock that contains NSURL* param
  • deprecated block type SDWebImageCompletedBlock, replaced with SDWebImageCompletedBlock that contains NSURL* param
  • deprecated SDWebImageManager -downloadWithURL:options:progress:completed: method. Replaced with downloadImageWithURL:options:progress:completed: that uses the SDWebImageCompletionWithFinishedBlock as completion block type
  • created Deprecated category for SDWebImageManager containing the old method
  • deprecated all MKAnnotationView(WebCache) setImage* methods. Replaced with loadImage* methods that use the SDWebImageCompletionBlock as completion block type
  • created WebCacheDeprecated category on MKAnnotationView (to avoid collisions, we didn't name it Deprecated)
  • deprecated all UIButton(WebCache) setImage* methods. Replaced with loadImage* methods that use the SDWebImageCompletionBlock as completion block type
  • created WebCacheDeprecated category on UIButton (to avoid collisions, we didn't name it Deprecated)
  • deprecated all UIImageView(HighlightedWebCache) setImage* methods. Replaced with loadImage* methods that use the SDWebImageCompletionBlock as completion block type
  • created HighlightedWebCacheDeprecated category on UIImageView (to avoid collisions, we didn't name it Deprecated)
  • deprecated all UIImageView(WebCache) setImage* methods. Replaced with loadImage* methods that use the SDWebImageCompletionBlock as completion block type
  • created WebCacheDeprecated category on UIImageView (to avoid collisions, we didn't name it Deprecated)
  • replaced the usages of the deprecated items with the new ones
  • updated documentation, fixed misspells, re-aligned doc

Also managed to get rid of the naming collision with the AFNetworking UIImageView category methods. Now we use loadImageWithURL: instead of setImageWithURL:.

Fixes: #350, #722

PS: many thanks @ilushka85 for doing the heavy lifting on this

- deprecated block type `SDWebImageCompletedWithFinishedBlock`, replaced with `SDWebImageCompletionWithFinishedBlock` that contains NSURL* param
- deprecated SDWebImageManager `-downloadWithURL:options:progress:completed:` method. Replaced with `downloadImageWithURL:options:progress:completed:` that uses the `SDWebImageCompletionWithFinishedBlock ` as completion block type
- created Deprecated category for SDWebImageManager containing the old method
- replaced the usages of the deprecated items with the new ones
- created block type `SDWebImageCompletionBlock` that contains NSURL* param
- deprecated all MKAnnotationView(WebCache) `setImage*` methods. Replaced with `loadImage*` methods that use the `SDWebImageCompletionBlock` as completion block type
- created WebCacheDeprecated category on MKAnnotationView (to avoid collisions, we didn't name it Deprecated)
- replaced the usages of the deprecated items with the new ones
- deprecated all UIButton(WebCache) `setImage*` methods. Replaced with `loadImage*` methods that use the `SDWebImageCompletionBlock` as completion block type
- created WebCacheDeprecated category on UIButton (to avoid collisions, we didn't name it Deprecated)
- replaced the usages of the deprecated items with the new ones
- deprecated all UIImageView(HighlightedWebCache) `setImage*` methods. Replaced with `loadImage*` methods that use the `SDWebImageCompletionBlock` as completion block type
- created HighlightedWebCacheDeprecated category on UIImageView (to avoid collisions, we didn't name it Deprecated)
- replaced the usages of the deprecated items with the new ones
- deprecated all UIImageView(WebCache) `setImage*` methods. Replaced with `loadImage*` methods that use the `SDWebImageCompletionBlock` as completion block type
- created WebCacheDeprecated category on UIImageView (to avoid collisions, we didn't name it Deprecated)
- replaced the usages of the deprecated items with the new ones
- deprecated block type `SDWebImageCompletedBlock `, replaced with `SDWebImageCompletedBlock ` that contains NSURL* param
@bpoplauschi bpoplauschi added this to the 3.7.0 milestone Jun 19, 2014
@bpoplauschi
Copy link
Member Author

@rs please take a good look at this. I think it's all good, but you know this code better than anyone. Thanks in advance.

@Whirlwind
Copy link
Contributor

I think this is a good pull request.

@rs
Copy link
Member

rs commented Jun 19, 2014

Why do we change the whole API and add so much burden just to add a param to the callback?

I would be more comfortable by keeping the lib clean of deprecated code and just make the backward incompatible change in a major release. It won't break dependent apps as they won't compile and force devs to update their code.

We may carefully document this change to help people migrate their code easily.

@ilushka85
Copy link

Why wait for a major release.... when we can do this... deprecate old
methods and in a major release clean the code of deprecated methods.

On Thu, Jun 19, 2014 at 11:17 AM, Olivier Poitrey notifications@github.com
wrote:

Why do we change the whole API and add so much burden just to add a param
to the callback?

I would be more comfortable by keeping the lib clean of deprecated code
and just make the backward incompatible change in a major release. It won't
break dependent apps as they won't compile and force devs to update their
code.

We may carefully document this change to help people migrate their code
easily.


Reply to this email directly or view it on GitHub
#770 (comment).

Kind regards,
Ilya Beyrak | Founder & CEO
Resultly LLC.
(p) 312.273.9401
(m) 847.912.1212
(w) www.result.ly

@rs
Copy link
Member

rs commented Jun 19, 2014

What about breaking it now and say the next release is a major? I really don't like the idea to change method names semantic just because we introduce a change in the rest of the method's prototype.

@rs
Copy link
Member

rs commented Jun 19, 2014

At least, if you really need to do that, don't change the semantic and add a prefix to the category methods (i.e.: sd_setImageWithURL…). That would fix a conflict with AFNetworking UIImage category who copied our method names and would be more future proof.

@bpoplauschi
Copy link
Member Author

@rs I totally understand your point about keeping the library clean. But I don't think what we have right now for the next version qualifies for a major release (other than if we were breaking compatibility). I think the approach you suggested (prefixing the method names) is great and I vote for that. I will go ahead and update my pull request. Please let me know if you disagree so I don't do that.

@bpoplauschi
Copy link
Member Author

I updated with the sd prefixes for all the category methods. Please review

@rs
Copy link
Member

rs commented Jun 19, 2014

That's better. We now have to make sure we clean those deprecated methods in the next major otherwise it will still conflict with AFNetworking. This should then fix #350.

@bpoplauschi
Copy link
Member Author

I created a 4.0.0 milestone and issue #774 set on 4.0.0 that will remind us to clean those up. Could you merge it now?

@rs
Copy link
Member

rs commented Jun 19, 2014

Yes

rs pushed a commit that referenced this pull request Jun 19, 2014
@rs rs merged commit a8a69ab into SDWebImage:master Jun 19, 2014
@bpoplauschi
Copy link
Member Author

Thank you

@bpoplauschi bpoplauschi deleted the completion_with_url branch June 19, 2014 19:40
@ilushka85
Copy link

Thanks guys

On Thursday, June 19, 2014, Bogdan Poplauschi notifications@github.com
wrote:

Thank you


Reply to this email directly or view it on GitHub
#770 (comment).

Kind regards,
Ilya Beyrak | Founder & CEO
Resultly LLC.
(p) 312.273.9401
(m) 847.912.1212
(w) www.result.ly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Name collisions in UIImageView category
4 participants