-
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
Adding macOS support for Messaging and InstanceID #2880
Conversation
Please resolve style issues. |
…update keychain parameters for macOS
Seeing tvOS build errors - firebase-ios-sdk/Firebase/InstanceID/FIRInstanceID.m:1183:8: error: use of undeclared identifier 'apsEnvironment' |
…] enabled for macOS
…into fcm-iid-macos
…into fcm-iid-macos
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.
Update .travis.yml to run InstanceID for macos (two places)
Also, if you like, the architecture flags may no longer be necessary after Maksym's stability fixes.
The architecture flags are the updated version, I believe, but I can let Maksym test again to see if some of them can be removed. Regarding the error: https://travis-ci.org/firebase/firebase-ios-sdk/jobs/538889246#L1757 I have to manually set "Weak References in Manual Retain Release" to true under the FirebaseMessaging-MacOS-Unit-Unit pod, I wonder what I need to do in podspec to reflect such flag automatically so travis can pass green? |
You can set options for the test target's build config here - https://github.com/firebase/firebase-ios-sdk/blob/master/FirebaseMessaging.podspec#L56. Out of curiosity, what's special about macos that it needs that flag? |
So that's the weird part: for iOS and tvOS, xcode is default "true", for macOS it's default "false". |
There are some breakage but none of them seem related to fcm/iid. |
I see two issues - one might be an iOS flake, but the second seems like real tvOS issues. In https://travis-ci.org/firebase/firebase-ios-sdk/jobs/538966556: Failing tests: In https://travis-ci.org/firebase/firebase-ios-sdk/jobs/538966560
|
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.
Nice!
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.
👍
With this change, developers can use FCM to send push notifications on macOS app.