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 geolocalization disabled by default #157

Open
heitortsergent opened this issue May 16, 2016 · 6 comments · May be fixed by #172
Open

Make geolocalization disabled by default #157

heitortsergent opened this issue May 16, 2016 · 6 comments · May be fixed by #172

Comments

@heitortsergent
Copy link
Contributor

https://github.com/keenlabs/KeenClient-iOS/blob/master/KeenClient/KeenClient.m#L194

@terrhorn
Copy link
Contributor

Documentation will need to be updated when this change occurs as it specifically states that Geo Location is enabled by default: https://github.com/keenlabs/KeenClient-iOS#geo-location

@heitortsergent heitortsergent linked a pull request Sep 13, 2016 that will close this issue
@terrhorn
Copy link
Contributor

Concern: this is going to break existing implementations that don't explicitly enable Geo Location, perhaps unknowingly so. Do we have a good reason for disabling by default vs. enabling by default?

@heitortsergent
Copy link
Contributor Author

I think there are a couple of good reasons why disabled by default is a better behavior:

  1. That change means we are asking developers to be more conscious about their user's privacy, and explicit about needing location information.
  2. It can save user's bandwidth (maybe it's negligible) and resources for Keen.

Yes, this will break existing implementations. I don't know if those 2 reasons are good enough taking that into account, and we would definitely need to notify the developers that use this SDK, and maybe give a timeline of when this will be merged. But I think we should make that change eventually.

@terrhorn
Copy link
Contributor

The challenge in notifying those who use this particular SDK is that none of our SDKs currently include any data that would allow us to discern one SDK from another. We should leave this PR open until we've added that capability.

@heitortsergent
Copy link
Contributor Author

heitortsergent commented Sep 14, 2016

Ok, sounds good 👍 . I'll open a separate issue for adding that capability.

@baumatron
Copy link
Contributor

#173 has been completed, so this should be good to go in. Does seem like it is a breaking change, so maybe should be slated for major release.

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

Successfully merging a pull request may close this issue.

3 participants