-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add core support with interop for Segmentation SDK. #3430
Conversation
…rs to be under sources folder.
Firebase/Core/FIRErrors.m
Outdated
@@ -19,3 +19,4 @@ | |||
NSString *const kFirebaseCoreErrorDomain = @"com.firebase.core"; | |||
NSString *const kFirebasePerfErrorDomain = @"com.firebase.perf"; | |||
NSString *const kFirebaseStorageErrorDomain = @"com.firebase.storage"; | |||
NSString *const kFirebaseSegmentationErrorDomain = @"com.firebase.segmentation"; |
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.
Delete. FIRErrors is deprecated and nothing new should be added.
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.
Done.
Firebase/Core/Private/FIRErrors.h
Outdated
@@ -22,3 +22,4 @@ extern NSString *const kFirebaseErrorDomain; | |||
extern NSString *const kFirebaseConfigErrorDomain; | |||
extern NSString *const kFirebaseCoreErrorDomain; | |||
extern NSString *const kFirebasePerfErrorDomain; | |||
extern NSString *const kFirebaseSegmentationErrorDomain; |
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.
Delete. Same as above.
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.
Done.
#import "FIRSegmentation.h" | ||
#import "FirebaseSegmentation/Sources/Public/FIRSegmentation.h" | ||
|
||
#import "FIRAppInternal.h" |
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.
Should be #import <FirebaseCore/FIRAppInternal.h> for this and other Core Private headers.
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.
Done.
@@ -0,0 +1,86 @@ | |||
#import "FIRSegmentationComponent.h" | |||
|
|||
#import "FIRAppInternal.h" |
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.
Use module import format here too.
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.
Done.
|
||
@end | ||
|
||
/// A concrete implementation for FIRSegmentationInterop to create Segmentation instances and |
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.
Is there a need for FIRSegmentationInterop?
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.
Is there a reason not to? What is the alternative? I thought all products were moving to using interop.
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.
Interop is only needed for products that want to manage their interface to other Firebase dependencies separately from their library implementation. Will Segmentation have Firebase pod dependencies?
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.
Yes, Segmentation will need to depend on IID and FIS.
FirebaseSegmentation.podspec
Outdated
@@ -21,7 +21,7 @@ Firebase Segmentation enables you to associate your custom application instance | |||
s.prefix_header_file = false | |||
|
|||
s.source_files = 'FirebaseSegmentation/Sources/**/*' | |||
s.public_header_files = 'FirebaseSegmentation/Public/*.h' | |||
s.public_header_files = 'FirebaseSegmentation/Sources/Public/*.h' | |||
|
|||
s.dependency 'FirebaseCore', '~> 6.0' |
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.
6.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.
Done.
|
||
@end | ||
|
||
/// A concrete implementation for FIRSegmentationInterop to create Segmentation instances and |
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.
Interop is only needed for products that want to manage their interface to other Firebase dependencies separately from their library implementation. Will Segmentation have Firebase pod dependencies?
Please fix the travis failure before merging. |
Yes, segmentation will have IID and FIS as pod dependencies. |
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.
Updating travis.yml is not a good idea since it forces all jobs to run instead of just the ones impacted by the change.
Add core support with interop for Segmentation SDK. Also update header file location to within sources folder.