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

Initial stale logic added #264

Merged
merged 4 commits into from
Feb 13, 2018
Merged

Initial stale logic added #264

merged 4 commits into from
Feb 13, 2018

Conversation

cammace
Copy link

@cammace cammace commented Jan 29, 2018

Closes #58

@cammace cammace self-assigned this Jan 29, 2018
@cammace cammace requested a review from tobrun January 29, 2018 21:48
@cammace cammace added ready for review When your PR has been personally reviewed, its time for an external contributors to approve and removed ⚠️ DO NOT MERGE labels Feb 8, 2018
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good, just one question about the singleton setup

private Handler handler;
private long delayTime = DEFAULT_DELAY_TIME_TILL_STALE;

static StaleStateRunnable getInstance() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there are reason why this class is a singleton? can't we get away with using a final field in the plugin instead?


private StaleStateRunnable() {
onLocationStaleListeners = new ArrayList<>();
handler = new Handler();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these can be directly initialised as part of the field declaration + they can be made final

Cameron Mace added 3 commits February 13, 2018 12:34
* adds location layer options class

* Fixed memory leak in manual location update activity

* Remove stale state listener in onStop

* add elevation attribute

* fix accuracy showing during nav mode

* added documentation to location options

* added some test for the class

* fixed test

* set locationlayermode at beginning of method

* remove toggleCameraListener param
@cammace cammace merged commit 57c972c into master Feb 13, 2018
@cammace cammace deleted the stale-state branch February 13, 2018 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review When your PR has been personally reviewed, its time for an external contributors to approve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants