WorryFree Computers   »   [go: up one dir, main page]

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

Add core support with interop for Segmentation SDK. #3430

Merged
merged 10 commits into from
Jul 25, 2019
Merged

Conversation

dmandar
Copy link
Contributor
@dmandar dmandar commented Jul 24, 2019

Add core support with interop for Segmentation SDK. Also update header file location to within sources folder.

@@ -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";
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -22,3 +22,4 @@ extern NSString *const kFirebaseErrorDomain;
extern NSString *const kFirebaseConfigErrorDomain;
extern NSString *const kFirebaseCoreErrorDomain;
extern NSString *const kFirebasePerfErrorDomain;
extern NSString *const kFirebaseSegmentationErrorDomain;
Copy link
Member

Choose a reason for hiding this comment

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

Delete. Same as above.

Copy link
Contributor Author

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"
Copy link
Member

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.

Copy link
Contributor Author

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"
Copy link
Member

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.

Copy link
Contributor Author

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
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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.

@@ -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'
Copy link
Member

Choose a reason for hiding this comment

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

6.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@dmandar dmandar requested a review from paulb777 July 25, 2019 05:40
FirebaseSegmentation/Sources/FIRSegmentationComponent.m Outdated Show resolved Hide resolved

@end

/// A concrete implementation for FIRSegmentationInterop to create Segmentation instances and
Copy link
Member

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?

@paulb777
Copy link
Member

Please fix the travis failure before merging.

@dmandar
Copy link
Contributor Author
dmandar commented Jul 25, 2019

Yes, segmentation will have IID and FIS as pod dependencies.

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.

Updating travis.yml is not a good idea since it forces all jobs to run instead of just the ones impacted by the change.

@dmandar dmandar merged commit b3df79f into md-floc-master Jul 25, 2019
@morganchen12 morganchen12 deleted the md-floc-1 branch August 1, 2019 20:57
@firebase firebase locked and limited conversation to collaborators Oct 11, 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.

None yet

3 participants