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

Use AFNetworking instead of NSURLSession, thus we have iOS 6 support #17

Closed
wants to merge 3 commits into from

Conversation

shkutkov
Copy link

Thank you for Haneke! It rocks!
But many apps are still support iOS 6, so it'll be great to have such support in Haneke. By using AFNetworking we can do it truly simple. What do you think?

@hpique
Copy link
Member

hpique commented Apr 11, 2014

Thanks @shkutkov!

As you mentioned making Haneke support iOS 6 is quite straightforward. The reason I didn't do it is because I developed Haneke for my iOS 7 apps.

What you did is great, and I like that you took the extra care to update the unit tests. Two caveats:

  • Yesterday I committed code that adds image decompression using kCGImageSourceShouldCacheImmediately, which is iOS 7 only. For iOS 6 you could use the CGContextDrawBitmap method (example) or simply don't do anything and make hnk_decompressedImageWithData return:
[UIImage imageWithData:data scale:[UIScreen mainScreen].scale];
  • Personally, I would prefer avoiding the AFNetworking dependency and instead use NSURLConnection when running on iOS 6. I use AFNetworking too but I prefer my pods to require as few dependencies as possible.

In any case, I will add a note in the readme for those who are looking for iOS 6, linking to your fork.

@nullproduction
Copy link
Contributor

Thanks @shkutkov.
I think it will be useful

@shkutkov
Copy link
Author

Thank you Hermes.

  • I haven't seen your commit with kCGImageSourceShouldCacheImmediately yesterday. Personally I'd prefer to keep things simple and just have a stub in iOS 6.
  • As for dependencies. I agree that avoiding dependency is a good decision, but it might take just a little bit more time and have an impact on code complexity.

Anyway, keep up the good work!

@hpique
Copy link
Member

hpique commented Apr 11, 2014

I'm closing the pull request as it won't be merged and the issue is now linked from the readme. Feel free to continue the conversation.

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

Successfully merging this pull request may close these issues.

None yet

4 participants