-
Notifications
You must be signed in to change notification settings - Fork 299
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
Reachability notification name clashes with the Apple example #359
Comments
Hi @grigorye Thanks for reporting this. Makes sense. Will be changed in upcoming v4.17.0 release of our SDK. Will keep you posted in here on update progress. Cheers |
Thanks, @uerceg! Beware though that it might affect clients that happen to (implicitly) depend on Adjust triggering that very notification, due to e.g. a bug in their own reachability tracking. In other words, it might be a "breaking change in case for misbehaving clients". |
Of course and thanks for the comment. It will definitely be mentioned in release notes and |
ios_sdk/Adjust/ADJReachability.m
Line 20 in e207090
kADJReachabilityChangedNotification
is defined as"kNetworkReachabilityChangedNotification"
. This presents a potential issue for any code that integrates Adjust and uses Reachability.m as defined by Apple.The issue is that any client code that relies on Apple example for Reachability and that observes
all
"kNetworkReachabilityChangedNotification"
notifications (e.g. passingnil
asobject
to-[NSNotificationCenter addObserver:forName:object:queue:usingBlock:]
will get extra notification from Adjust (beware that the client and Adjust might or might not observe different reachability configurations, depending on the situation).I suggest to change the notification value to something different (like
"kADJReachabilityChangedNotification"
) to avoid the potential clash.For the reference, there're quite a few mentions of
"kNetworkReachabilityChangedNotification"
in the Rechability-related examples posted on the Internet, and at quick glance, it's not mentioned anywhere that it's better to use client-specific notification name.https://www.google.nl/search?q=knetworkreachabilitychangednotification
The text was updated successfully, but these errors were encountered: