-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Created tvOS Example Pack #141
Conversation
@Sherlouk updated the pull request - view changes |
@Sherlouk updated the pull request - view changes |
@Sherlouk updated the pull request - view changes |
Having a few issues with the travis change, wasn't working locally when I tried but a few retrys later it worked locally. Would appreciate some help with that! |
@Sherlouk It looks like Travis is timing out from a build issue. You should be able to run this locally:
That's the command its timing out on (+ Also I've literally never built an Apple TV app, so I'm pretty n00b when it comes to that 🙊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Sherlouk !
Let's mark all these new classes final
, too
|
||
script: | ||
- if [ $POD_LINT == "YES" ]; then | ||
bundle exec pod lib lint; | ||
fi | ||
|
||
|
||
- if [ $BUILD_EXAMPLE == "YES" ]; then | ||
- if [ $BUILD_IOS_EXAMPLE == "YES" ]; then | ||
xcodebuild build -workspace Example/IGListKitExamples.xcworkspace -scheme IGListKitExamples -sdk "$SDK" -destination "$DESTINATION" ONLY_ACTIVE_ARCH=NO CODE_SIGNING_REQUIRED=NO | bundle exec xcpretty -c; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's revert these changes. Instead, inside of this if
we can check the value of $SDK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, much better idea! Admittedly not my strong point!
|
||
PODFILE CHECKSUM: 4bdfb42d1e7b2b121bf8c83bc300d7d77aa0fc3a | ||
|
||
COCOAPODS: 1.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use 1.1.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left it as default, will change
With regards to Travis, definitely not my strong point but will try and get it working (will make the suggested changes first!) Definitely happy with making things final and updating the cocoapods version. Are we satisfied with the demo I chose to do/the chooser I implemented? I'm actually with you @rnystrom I've actually never done tvOS hence why I wanted to do this to try and learn something new :) (Had fun learning how to focus cells and doing all that!) |
@Sherlouk updated the pull request - view changes |
Travis hasn't run yet, but I haven't changed the build command so I doubt the result will change. When I ran the build command locally it didn't work at first, it stalled because I had a different build of xcode open than it wanted to use. It was forcing itself to use xcode 8 but I had 8.beta2 open. I closed it, and reran the command and it worked perfectly. I've updated my local version of cocoapods to 1.1.1, made classes final and made the travis file a bit cleaner as per requests. |
1 similar comment
As much as I'd love to say I fixed it -- I broke the build shell script. 🙈 Working on it |
@Sherlouk updated the pull request - view changes |
Failed in same place, but no closer to finding the actual issue. As I mentioned previously working fine when running locally, so let's just ship my machine! But in all seriousness, any ideas @rnystrom ? |
1 similar comment
xcodebuild build -workspace Examples-tvOS/IGListKitExamples.xcworkspace -scheme IGListKitExamples -sdk "$SDK" -destination "$DESTINATION" ONLY_ACTIVE_ARCH=NO CODE_SIGNING_REQUIRED=NO | bundle exec xcpretty -c; | ||
fi | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@Sherlouk -- are the tvOS example schemes shared? |
@Sherlouk updated the pull request - view changes |
@jessesquires Yeah, obviously, who would forget to do that? Insane idea! (oops) |
1 similar comment
That was slightly embarrassing. 😓 Thanks Jesse! |
@Sherlouk what's the reasoning behind another sample project vs adding this all to the existing one? IMO I'd like to keep the top-level dir as trim as possible (which is becoming tough). Can we not add this to the existing sample project? Maybe I missed something? |
@rnystrom It originally was all in one project file, but redid it all in a fresh project file as per @jessesquires' comments (#138 (comment)) Edit:// I have no real preference, but I do think one project file might get a bit unmanageable over time especially with things like Today/iMessage schemes which only work on iOS etc etc |
Any note on what I need to do to move this forward? |
@rnystrom -- Yeah, I think we should have a separate tvOS example project. Let's keep directories how they are for this PR and follow up after. Here's what I propose:
|
# Conflicts: # CHANGELOG.md
@Sherlouk updated the pull request - view changes |
@jessesquires Not a problem, was waiting for everybody to be happy before fixing it as other PRs kept making it conflict! Should all work now 🙏 |
Let's land it! 🚀 🚀 🚀 |
1 similar comment
@rnystrom has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Changes in this pull request
travis.yml
to optionally build tvOS example packNote: Not added any imagery/icons/etc due to the much larger & different sizes required - If they're desired then someone with skills will need to provide them!
Closes #138
Relates to #137
Pull request checklist