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.

Sign up for GitHub

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

Jump to bottom

tvOS support #570

Closed
wants to merge 15 commits into from
Closed

tvOS support #570

wants to merge 15 commits into from

Conversation

justindhill
Copy link

This PR addresses #440. It fixes compilation issues under tvOS as well as re-adding NSUserDefaults storage of the trait collection and event queue under tvOS. We check for the file on disk as normal, then fall back onto the NSUserDefaults entry if it doesn't exist. In addition, the Podspec was updated with a version bump to 3.2.6 and a new deployment target for tvOS (9.0).

Couple of things:

  1. I think it would actually be better to simply store the event queue and traits entirely in NSUserDefaults on tvOS and eschew the file representation altogether. I didn't do this because @f2prateek expressed a desire to only fall back on NSUserDefaults in TvOS Support #440.
  2. I've never used Carthage, but I looked into it and it appears that you must have a separate target in your Xcode project for each device target, as it doesn't do any project generation. This presents an issue with the current project structure - analytics-ios uses the development pod target created by CocoaPods and has no explicit target, so I would have had to significantly alter the structure, and I wasn't feeling particularly presumptuous, so I left it alone. :)

f2prateek and others added 11 commits June 30, 2016 08:31
@f2prateek
Carthage Support
4b03a75
@f2prateek
Initialize flush timer on the main thread.
3b95cea
@indragiek @f2prateek
Replace Foundation import with UIKit import (#563)
5ce238f
@f2prateek
Track Campaign Data (#557)
fcf8725
@f2prateek
Update contributing docs (#564)
02414b0
@f2prateek
Limit Queue size to 1000 (#565)
8a1acca
@f2prateek
Reformat (#566)
3db0093
@f2prateek
Exclude files from backup (#567)
fbbfc3a
@f2prateek
correctly skip forwarding disabled events in the tracking plan (#568)
b28726a 
Previously events that were marked as disabled in the tracking plan
would still be sent to Segment (and other integrations, both client side
and server side).

The bug was that we were accidentally checking the class name of the
SEGTrackPayload instead of the `event` in the payload.
@justindhill
- Re-added NSUserDefaults persistence for traits and the event queue on
748f943 
  tvOS targets
@justindhill
- Fixed compilation errors on tvOS
cc4f924 
- Fixed a bug that caused an incomplete set of traits to be persisted to
  NSUserDefaults
- On tvOS, switched the order of loading of traits and the queue so it's
  attempted from the file first, then NSUserDefaults
- Updated the Podspec
  - Bumped the version to 3.2.6
  - Added a tvOS deployment target version (9.0)
@justindhill
Copy link
Author

It looks like the Travis build failed because the Xcode version selected for the project is too old - it doesn't contain the tvOS SDK. If you bump the version to Xcode 7.3, it should build fine.

f2prateek
f2prateek reviewed Jun 30, 2016
View reviewed changes
Analytics.podspec
@@ -1,6 +1,6 @@
Pod::Spec.new do |s|
s.name = "Analytics"
s.version = "3.2.5"
s.version = "3.2.6"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave this as is? We'll bump it as part of the release process.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@f2prateek
Copy link
Contributor

@justindhill Thanks! Some comments:

  1. Feel free to bump the Xcode version in the travis config.
  2. Sorry for the confusion - I do agree that we should skip the file persistence on tvOS altogether, and use NSUserDefaults instead. On iOS - we can leave the current behaviour as is.

@justindhill
Copy link
Author

After I bumped the Xcode version in Travis it looks like we timed out on [Info] Collecting info for testables..., but it passed on my machine!

image

@f2prateek
Copy link
Contributor

@justindhill no worries, I'll test this locally as well and we can skip waiting on the CI to pass (it's been a bit flaky recently).

Could you reopen this PR against dev instead of master as well — https://github.com/segmentio/analytics-ios/blob/dev/CONTRIBUTING.md

@justindhill
Copy link
Author

👍 re-opened as #571 into dev

@justindhill justindhill closed this Jul 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justindhill @f2prateek @indragiek