-
Notifications
You must be signed in to change notification settings - Fork 48
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
Remove nav event creation related codes #383
Conversation
Codecov Report
@@ Coverage Diff @@
## master #383 +/- ##
============================================
+ Coverage 62.04% 68.14% +6.09%
+ Complexity 517 348 -169
============================================
Files 98 63 -35
Lines 2967 2009 -958
Branches 208 150 -58
============================================
- Hits 1841 1369 -472
+ Misses 1026 552 -474
+ Partials 100 88 -12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Should we think about removing serializers:
mapbox-events-android/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClient.java
Line 164 in 6851cb8
private GsonBuilder configureGsonBuilder() { |
Can you please also add a link to the PR in navigation sdk repo for visibility?
@andrlee We already removed this method in this PR. |
@Chaoba if that's true then that might be a problem since these serializers have to be registered for nav events to work. |
@andrlee The reason why we need these serializers is because in nav related Events there will be a variable with type Since we will move all nav events to nav sdk, telemetry should not have any dependency on it and then we need to remove My solution is that we create a NavBaseEvent that will have variables in |
6c8a086
to
ace3a91
Compare
@andrlee This PR is good to go, will merge to master and test nav sdk with snapshot release. |
Fix #204
Remove nav event creation related code and corresponding test code.
Corresponding PR in nav sdk