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

Refactor event queuing logic #344

Merged
merged 8 commits into from
Feb 21, 2019
Merged

Refactor event queuing logic #344

merged 8 commits into from
Feb 21, 2019

Conversation

andrlee
Copy link
Contributor

@andrlee andrlee commented Feb 21, 2019

Refactor event queuing logic to address potential races with event queue: #342
Add more error handling and remove layers of redundant callback wrappers and simplifying internal housekeeping. Re-write unit tests and make them more DRY.

/cc @Guardiola31337 @angelanavarro

Fix instrumentations test time out for LocationEngine test.
@codecov
Copy link

codecov bot commented Feb 21, 2019

Codecov Report

Merging #344 into master will decrease coverage by 0.19%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##             master    #344     +/-   ##
==========================================
- Coverage     54.99%   54.8%   -0.2%     
+ Complexity      493     488      -5     
==========================================
  Files           106     103      -3     
  Lines          3266    3250     -16     
  Branches        219     217      -2     
==========================================
- Hits           1796    1781     -15     
+ Misses         1384    1383      -1     
  Partials         86      86

Copy link

@alfwatt alfwatt left a comment

Choose a reason for hiding this comment

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

Overall LGTM, good reduction in class count

@andrlee andrlee merged commit 36d851a into master Feb 21, 2019
@andrlee andrlee deleted the al-fix-queue-race branch February 25, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants