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

Make location block to return nullable dictionary #285

Closed
GRiMe2D opened this issue Sep 17, 2020 · 4 comments
Closed

Make location block to return nullable dictionary #285

GRiMe2D opened this issue Sep 17, 2020 · 4 comments

Comments

@GRiMe2D
Copy link

GRiMe2D commented Sep 17, 2020

Summary

Amplitude v7.0.0 added location block for providing user location manually. But location isn't available instantaneously. For some moment location will be unknown. But new added location block doesn't mark it nullable however it checks for nil while accepting it. Can't return nil in swift

typedef NSDictionary *_Nonnull (^AMPLocationInfoBlock)(void);

if ([_appliedTrackingOptions shouldTrackLatLng] && self.locationInfoBlock != nil) {
NSDictionary *location = self.locationInfoBlock();
if (location != nil) {
[apiProperties setValue:location forKey:@"location"];
}
}

Proposed

 NS_ASSUME_NONNULL_BEGIN
 
 typedef NSString *_Nonnull (^AMPAdSupportBlock)(void);
-typedef NSDictionary *_Nonnull (^AMPLocationInfoBlock)(void);
+typedef NSDictionary *_Nullable (^AMPLocationInfoBlock)(void);
@haoliu-amp
Copy link
Contributor

Thanks for reporting the issue!
We released v7.0.1 to address it!
https://github.com/amplitude/Amplitude-iOS/releases/tag/v7.0.1

@john-mejia
Copy link

@haoliu-amp would it be possible to call out breaking changes in future release notes? This caused a breaking change in our application and we had to dig through the PR/code to get an understanding of exactly what went wrong.

@haoliu-amp
Copy link
Contributor

@john-mejia Sorry for your trouble with new changes. We did mark it to be BREAKING CHANGES in v7.0.0. (We bumped it to major version 7 since it's not back compatible change.) https://github.com/amplitude/Amplitude-iOS/releases/tag/v7.0.0

@john-mejia
Copy link

woah,. my apologies - we jumped straight to .1 and missed the original update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants