-
Notifications
You must be signed in to change notification settings - Fork 1.7k
The instanceID sync API is deprecated. Implement the async instanceID API #3993
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
Conversation
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.
LGTM, deferring to Maksym for approval.
FIRLogInfo(kFIRLoggerRemoteConfig, @"I-RCN000022", @"Success to get iid : %@.", | ||
instanceIDStrongSelf->_settings.configInstanceID); | ||
} else { | ||
FIRLogWarning(kFIRLoggerRemoteConfig, @"I-RCN000055", @"Error getting iid : %@.", |
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 it possible for !error && !identity? If so, this will crash.
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 it is..not sure where we would crash though..
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.
@dmandar Thank you for updating it!
Mostly LGTM, just one question in the comments.
__weak RCNConfigFetch *instanceIDSelf = strongSelf; | ||
[instanceID getIDWithHandler:^(NSString *_Nullable identity, NSError *_Nullable error) { | ||
RCNConfigFetch *instanceIDStrongSelf = instanceIDSelf; | ||
instanceIDStrongSelf->_settings.configInstanceID = identity; |
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 _settings
be modified from the _lockQueue
? [instanceID getIDWithHandler:]
is called on the main queue. I wonder if the code will be simpler if we call [instanceID getIDWithHandler:]
first (before line 209) and move the code with dispatch_async(instanceIDHandlerSelf->_lockQueue
inside the getIDWithHandler:
handler?
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.
Good pt. Done.
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.
LGTM
No description provided.