Merge FLoC SDK into master branch. #6466

diwu-arete · 2020-09-15T17:07:02Z

Context:
design doc:
https://docs.google.com/document/d/1Lh4cZ5RJieRh6sso-JqDA6xxJFrrX769BbpI__UbQGA/edit#heading=h.8lfbj9ruhrwi

api council approval:
https://docs.google.com/document/d/1Tw-1xqA3pGkZZVF4SW_P9cGaYdU9HyCXT0W7gYJSbDA/edit?ts=5ce5fdb4#heading=h.x94y9zzc13xn

* Create Segmentation SDK structure for source and unit tests. * Review changes: Remove unnecessary files in test folder. Add md-floc-master to CI * Rename Segmentation directory to FirebaseSegmentation directory. Update podspec to include search header path.

* Add core support with interop for Segmentation SDK. Also update headers to be under sources folder. * Review fixes. * Minor changes. * Fix style. * Fix style. * Style changes. * Fix whitespace in travis.yml * Fix style. * Travis CI is stuck..try updating the travis.yml * Undo travis.yml change.

…4574) * Working drop of Segmentation SDK along with sample app and unit tests. * Update if_changed.sh to include FirebaseSegmentation. * Complete NS_ASSUME_NON_NULL_START with NS_ASSUME_NON_NULL_END in header file. * Fix unit tests. * Fix style. * Fixes after running XCode's static analyzer. * Fix style. * fix style. * 'pod lib lint' fixes. * Fix analyzer errors. * Address review comments. * Minor changes for review comments. * Address review comments. * Address review comments. * stop mocking in tear down method for tests. * Minor update to sample app project. * Fix trailing whitespace in Podfile. * Add set -x to check.sh

google-oss-bot · 2020-09-15T17:10:30Z

Danger has errored

[!] Invalid Dangerfile file: syntax error, unexpected tCONSTANT, expecting )
...= hasChangesIn("FirebaseStorage/")
... ^~~~~~~~~~~~~~~
Dangerfile:109: syntax error, unexpected tCONSTANT, expecting end-of-input
...= hasChangesIn("FirebaseStorage/Sources/Public/")
... ^~~~~~~~~~~~~~~. Updating the Danger gem might fix the issue. Your Danger version: 6.1.0, latest Danger version: 8.0.5

 #  from Dangerfile:108
 #  -------------------------------------------
 #  @has_segmentation_api_changes = hasChangesIn("FirebaseSegmentation/Source/Public/
 >  @has_storage_changes = hasChangesIn("FirebaseStorage/")
 #  @has_storage_api_changes = hasChangesIn("FirebaseStorage/Sources/Public/")
 #  -------------------------------------------

Generated by 🚫 Danger

diwu-arete · 2020-09-15T20:41:59Z

/retest

…q-floc

morganchen12

Don't forget to update the Dangerfile with segmentation paths: https://cs.opensource.google/firebase-sdk/firebase-ios-sdk/+/master:Dangerfile;l=73?q=Dangerfile

FirebaseSegmentation.podspec

FirebaseSegmentation/Sources/FIRSegmentation.m

FirebaseSegmentation/Sources/FIRSegmentationComponent.m

FirebaseSegmentation/Sources/Public/FIRSegmentation.h

FirebaseSegmentation/Tests/Sample/Podfile

FirebaseSegmentation/Tests/Sample/SegmentationSampleApp/AppDelegate.m

FirebaseSegmentation/Sources/Public/FIRSegmentation.h

paulb777

GHA actions should be set up

FirebaseSegmentation.podspec

FirebaseSegmentation/Sources/FIRSegmentation.m

FirebaseSegmentation.podspec

FirebaseSegmentation/Tests/Sample/GoogleService-Info.plist

scripts/check.sh

morganchen12

You may need to update the copyright header.

FirebaseSegmentation/Sources/FIRSegmentationComponent.m

FirebaseSegmentation/Sources/Public/FIRSegmentation.h

morganchen12 · 2020-09-30T21:47:19Z

FirebaseSegmentation/Sources/SEGContentManager.m

+
+  _installationIdentifier = identifier;
+
+  __weak SEGContentManager *weakSelf = self;


Using __weak here is unnecessary since self is a singleton instance, and will thus never be deallocated.

FirebaseSegmentation/Sources/SEGContentManager.m

FirebaseSegmentation/Sources/SEGDatabaseManager.m

FirebaseSegmentation/Sources/SEGNetworkManager.m

morganchen12 · 2020-10-01T00:38:58Z

FirebaseSegmentation/Sources/SEGNetworkManager.m

+                    FIRLogError(kFIRLoggerSegmentation, @"I-SEG000022",
+                                @"Internal error making network request.");
+                    completionHandler(
+                        NO, @{@"errorDescription" : @"Internal error making network request."});


This errorDescription key is inconsistent with other completion handler invocations (errorDescription vs ErrorDescription). Can you use a global constant key instead of a string literal in every completion handler?

FirebaseSegmentation/Sources/SEGNetworkManager.m

morganchen12 · 2020-10-01T00:45:50Z

FirebaseSegmentation/Sources/SEGSegmentationConstants.h

+#import <Foundation/Foundation.h>
+
+#ifndef SEGSegmentationConstants_h
+#define SEGSegmentationConstants_h


nit: Header guards not required for files included with #import

FirebaseSegmentation/Sources/SEGSegmentationConstants.m

FirebaseSegmentation/Sources/Public/FIRSegmentation.h

morganchen12 · 2020-10-01T18:42:58Z

FirebaseSegmentation/Sources/SEGDatabaseManager.m

  dispatch_async(_databaseOperationQueue, ^{
-    SEGDatabaseManager *strongSelf = weakSelf;
+    SEGDatabaseManager *strongSelf = self;


You can remove this declaration (and the if statement below) and use self directly.

paulb777

Also, needs a rebase or merge with master

.travis.yml

scripts/if_changed.sh

paulb777

LGTM on green CI. Thanks!

dmandar and others added 7 commits May 15, 2020 15:52

update segmetation dependency version

33a0579

migrate FloC SDK to depend on FIS SDK directly

e6b434c

format floc

8e06a28

merge conflict

b6eaf23

googlebot added the cla: yes label Sep 15, 2020

diwu-arete requested review from ChaoqunCHEN and ryanwilson September 15, 2020 17:07

google-oss-bot added api: installations public-api-change labels Sep 15, 2020

format

fabd519

Merge branch 'master' of github.com:firebase/firebase-ios-sdk into cc…

20f4eb3

…q-floc

morganchen12 reviewed Sep 16, 2020

View reviewed changes

paulb777 requested changes Sep 17, 2020

View reviewed changes

ChaoqunCHEN added 12 commits September 25, 2020 11:35

using customized FIRapp

bd017c4

merge master

aa530db

Merge branch 'segmentation-master' into ccq-floc

aa58060

remove test plist file

f2e331b

Merge branch 'master' into segmentation-master

9a3b226

Merge branch 'master' into ccq-floc

a8ab414

Merge branch 'segmentation-master' into ccq-floc

7bd81cf

refactor to capture weakself

701c7a1

format

855f2d4

replace partial mock with class mock

564bff2

minor refactoring

dc215c7

use subscript to manipulate dictionary instance

278132b

fix import error

a824c21

ChaoqunCHEN requested a review from morganchen12 September 30, 2020 21:27

morganchen12 reviewed Oct 1, 2020

View reviewed changes

minor refoctoring, addressing comments

8c9f5e4

morganchen12 reviewed Oct 1, 2020

View reviewed changes

address comments

ad3aa80

ChaoqunCHEN requested a review from paulb777 October 1, 2020 20:25

paulb777 reviewed Oct 1, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

scripts/if_changed.sh Show resolved Hide resolved

ChaoqunCHEN added 2 commits October 1, 2020 14:26

fix configurations

ce13a34

Merge branch 'master' into ccq-floc

516e964

ChaoqunCHEN requested a review from paulb777 October 1, 2020 21:28

format

0a88ac5

paulb777 approved these changes Oct 1, 2020

View reviewed changes

ChaoqunCHEN merged commit 69bfe3f into master Oct 1, 2020

ChaoqunCHEN mentioned this pull request Oct 1, 2020

Firebase Segmentation SDK clean up and refactoring #6639

Closed

morganchen12 added the api: segmentation label Oct 1, 2020

paulb777 deleted the ccq-floc branch October 1, 2020 22:37

firebase locked and limited conversation to collaborators Nov 1, 2020


		_installationIdentifier = identifier;

		__weak SEGContentManager *weakSelf = self;

Merge FLoC SDK into master branch. #6466

Merge FLoC SDK into master branch. #6466

Uh oh!

Conversation

diwu-arete commented Sep 15, 2020

Uh oh!

google-oss-bot commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Danger has errored

Uh oh!

diwu-arete commented Sep 15, 2020

Uh oh!

morganchen12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulb777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morganchen12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

morganchen12 Sep 30, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morganchen12 Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

morganchen12 Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

morganchen12 Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

paulb777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paulb777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

google-oss-bot commented Sep 15, 2020 •

edited

Loading