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

Documentation and clarification around NYTPhoto.image vs .imageData #152

Merged
merged 4 commits into from
Feb 2, 2016

Conversation

cdzombak
Copy link
Contributor

This is mostly followup from the changes in #106.

  • 8ece787 cleans up one place where we could've seen inconsistent behavior in builds without animated GIF support (see commit message for details)
  • 043299b is a documentation improvement in NYTPhoto about image vs imageData
  • 34f45a1 adds a warning log message (in debug builds only) when you use the imageData property but animated GIF support is compiled out (since that really makes no sense, as discussed in Better UIImage Support #106)

@cdzombak
Copy link
Contributor Author

The warning introduced in 34f45a1 looks like this:

screen shot 2016-01-28 at 11 34 27

@bcapps
Copy link
Contributor

bcapps commented Jan 28, 2016

Do you think that it might also be worth renaming imageData to something like animatedImageData to hammer the point home both in general usage and for those who may skim over the docs?

@cdzombak
Copy link
Contributor Author

Do you think that it might also be worth renaming imageData to something like animatedImageData to hammer the point home both in general usage and for those who may skim over the docs?

@bcapps probably, but as that's a breaking API change it'll have to wait for v2. I'll file a separate issue under that milestone as a reminder, in the same vein as #136.

@bcapps
Copy link
Contributor

bcapps commented Jan 28, 2016

👍

@soundsokay
Copy link
Contributor

👍

cdzombak added a commit that referenced this pull request Feb 2, 2016
Documentation and clarification around NYTPhoto.image vs .imageData
@cdzombak cdzombak merged commit f790369 into develop Feb 2, 2016
@cdzombak cdzombak deleted the cdz/nytphoto-clarification branch February 2, 2016 19:15
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