Fix failing location activities#307
Conversation
|
@Guardiola31337, thanks for your PR! By analyzing the history of the files in this pull request, we identified @cammace and @zugaldia to be potential reviewers. |
zugaldia
left a comment
zugaldia
left a comment
There was a problem hiding this comment.
Just a small note to avoid ignoring a location value. Great call on using a WeakReference for the engines, could you bring that change to https://github.com/mapbox/mapbox-gl-native/blob/master/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/location/LocationSource.java#L39 as well?
| @Override | ||
| public void onLocationChanged(Location location) { | ||
| if (location != null) { | ||
| getLastLocation(); |
There was a problem hiding this comment.
@Guardiola31337 We're ignoring the location value passed to onLocationChanged here.
There was a problem hiding this comment.
Actually, we're not requesting location updates, so it's not necessary.
Problem. Are we going to leave onLocationChanged method empty? Should we segregate LocationEngineListener interface so client is not forced to depend on methods it does not use? What do you think? @zugaldia
There was a problem hiding this comment.
@Guardiola31337 It's only one method so it doesn't bring a lot of overhead, it keeps things simple. I'd leave it like it is right now. Also, it's more consistent with LocationListener this way.
|
@Guardiola31337 All good. If you want, and this is a minor thing, you could add a comment to the |
…not used on purpose
3 participants
Fixes #301
Added some
WeakReference's to avoid unwanted memory leaks.👀 @zugaldia