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

Added support for tvOS #131

Merged
merged 3 commits into from Mar 10, 2016
Merged

Added support for tvOS #131

merged 3 commits into from Mar 10, 2016

Conversation

voidrender
Copy link
Contributor

Added tvOS to the podspec, plus an example project with similar scenarios as the iOS example. Things seem to be working well, but there are two exceptions:

  1. FLAnimatedImage does not yet support tvOS. I opened a pull request to add it, but in the meantime, you can run the tvOS example by adding pod "FLAnimatedImage", :git => "https://github.com/ioveracker/FLAnimatedImage", :branch => "tvos" to the Podfile in Example-tvOS, above the line for PINRemoteImage.
  2. WebP doesn't seem to work--could be a limitation of the platform, or libwebp may need to be updated.

@garrettmoon
Copy link
Collaborator

@ioveracker awesome! I'll try and take a look at this as soon as I can!

@garrettmoon
Copy link
Collaborator

FYI, I'm releasing 2.0 very soon, but will make sure to release a 2.1 when we get this in :)

@garrettmoon
Copy link
Collaborator

@ioveracker I'm sorry this hasn't made it in yet, embarrassingly, I need to get our team on a newer version of cocoa pods which supports tvOS before I can merge. I'm going to work on that soon! 2.1 will be the tvOS release :)

@voidrender
Copy link
Contributor Author

No worries! The FLAnimatedImage pull request is still pending as well.

@garrettmoon
Copy link
Collaborator

@ioveracker Just finished upgrading the team to CocoaPods so I'm swinging back around to this. I downloaded your branch to give it a try, but I'm not able to get the example to run in Example-tvOS. I get an import error on PINRemoteImage every time.

I'm excited to have a Swift example and tvOS support landed :) Do you have any ideas what could be happening? I pod updated in the directory and that didn't work.

@voidrender
Copy link
Contributor Author

Hm, I just did a clean clone, updated the Podfile to use my FLAnimatedImage repo (since that pull request is still pending over at FLAnimatedImage), ran pod install, then opened the workspace and it built and started running on the tvOS simulator. I'm running Xcode 7.2.1 now, and was on whatever the latest was when I started this. :)

Since it's using the local files for PINRemoteImage, maybe there's some crossover between work you've been doing on the master branch? I don't know if that's possible, because it should be building it from source, and when switched to the tvos branch, none of the changes from the master branch should be in the source.

@voidrender
Copy link
Contributor Author

Crossover with the master branch is unlikely--I just pulled the latest master changes into my repo and even merged it into my local tvos branch and didn't encounter any issues. 😕

@garrettmoon
Copy link
Collaborator

Ok, I'll give it another try with a clean clone!

@voidrender
Copy link
Contributor Author

Coming back to this after a month, there are a couple things that I think could be changed (besides the fact that you can't get it to run! 😅) :

  1. The WebP example being the first tab isn't exactly ideal, since that's the only example that doesn't actually work on tvOS. 😆 Perhaps it would be better to move this example to the last tab.
  2. Since the FLAnimatedImage pull request is still pending, would you like to configure a way to build for tvOS without FLAnimatedImage so non-GIF support can be released separately, while FLAnimatedImage figures out their side of things? Or just wait for that to be merged before this gets merged?
  3. I noticed that the Pods directory was committed for the other examples. Would you like the Pods directory to be committed here, too? Will require deciding how to proceed with the FLAnimatedImage side of things first. 😁

@garrettmoon
Copy link
Collaborator

@ioveracker Cool! Maybe it will just work with the updates :P

  1. Yeah, that sounds good.
  2. I think we should configure support for without. Instead of including the PINRemoteImage pod spec, use PINRemoteImage/iOS? Make one for tvOS specifically?
  3. Yes, please commit the Pods directory, it makes it more reliable for using pod try

Thanks for you work on this!

@voidrender
Copy link
Contributor Author

Sounds good! I'll take a stab at these later this evening.

@voidrender
Copy link
Contributor Author

Okay, I've taken care of the above three items, and rebased the latest changes from master.

  1. I removed the GIF and WebP examples. It'll be easy to add them again if/when supported.
  2. I added a tvOS subspec. It's identical to the iOS one, but I thought it would be nice to start with a separate subspec in case they diverge.
  3. Pods directory committed.

I haven't been able to figure out why the Travis build is failing. Is it really being terminated because the log file is exceeding 4 MB just during the normal build, or is something else going on? I see a similar result in #308.1.

@garrettmoon
Copy link
Collaborator

@ioveracker ugh, yeah, travis has been a pain to get working correctly lately. Mind removing the verbose flag in the pod lint step in the .travis.yml file?

@garrettmoon
Copy link
Collaborator

Actually, I just removed it, so if you could just rebase, I'd appreciate it.

Isaac Overacker added 3 commits March 9, 2016 20:16
- tvOS subspec excludes FLAnimatedImage (for now)
- Removed GIF and WebP examples
- Added Pods directory
@voidrender
Copy link
Contributor Author

No problem! Done.

@garrettmoon
Copy link
Collaborator

It works!

@voidrender
Copy link
Contributor Author

🎉

garrettmoon added a commit that referenced this pull request Mar 10, 2016
@garrettmoon garrettmoon merged commit 75a485e into pinterest:master Mar 10, 2016
@garrettmoon
Copy link
Collaborator

Thanks again, this is awesome!

@voidrender
Copy link
Contributor Author

Sure thing! Thanks for all your hard work on PINRemoteImage, it's really great.

@voidrender voidrender deleted the tvos branch March 10, 2016 04:47
tinder-garricnahapetian pushed a commit to TinderApp/PINRemoteImage that referenced this pull request Mar 22, 2024
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

2 participants