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

Make FCM and InstanceID compatible with tvOS #2428

Merged
merged 11 commits into from
Mar 5, 2019
Merged

Conversation

charlotteliang
Copy link
Contributor

No description provided.

Example/Firebase.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
Firebase/Messaging/FIRMessaging.m Outdated Show resolved Hide resolved
@paulb777
Copy link
Member

A few more things I noticed trying to run in the branch:

  • GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS needs to be added to the Preprocessor Macros setting similar to how it is for the iOS Tests target
  • The Test scheme should be marked shared and visible in the scheme editor and then the resulting file added to the repo
  • Some extra cruft got into the HEADER_SEARCH_PATHS setting. Make it the same as the iOS test target.

@charlotteliang
Copy link
Contributor Author

There's conflict in the file, should I keep adding IID tests and merge later or just submit this CL first?

@paulb777
Copy link
Member

The conflict needs to be solved before merging. It looks feasible now after deintegrating. At a quick glance, it looks like "Accepting Both" for the one conflict will do it.

Also, the new test targets should be added to the "All tvOS" Aggregate target scheme and then they'll be included in CI.

@paulb777
Copy link
Member

Do git merge origin/master and git mergetool

@charlotteliang
Copy link
Contributor Author

Done merging the conflicts.

@charlotteliang
Copy link
Contributor Author

It doesn't let me merge by running git merge origin/master
So I did the step 1 of comment line instructions and push the change to this PR.

@paulb777
Copy link
Member

Looks like we need to update the FirebaseInstanceID version on SpecsStaging for FirebaseMessaging to pass pod lib lint.

To be safe, let's hold off on this until the M44 release branch gets made.

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

LGTM when travis is green

A follow up PR should add IID and tvOS unit tests

@charlotteliang charlotteliang merged commit 0016825 into master Mar 5, 2019
@paulb777 paulb777 deleted the fcm-tvos branch March 5, 2019 23:29
@firebase firebase locked and limited conversation to collaborators Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants