Null pointer fix #423
Merged
Null pointer fix #423
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
korshaknn
force-pushed
the
fix-crash-on-telemetry-client
branch
from
fda91cb
to
b76314e
Compare
korshaknn
force-pushed
the
fix-crash-on-telemetry-client
branch
from
b76314e
to
3f536f6
Compare
yunikkk
reviewed
Aug 15, 2019
libtelemetry/src/full/java/com/mapbox/android/telemetry/MapboxTelemetry.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
============================================
+ Coverage 68.23% 68.25% +0.01%
+ Complexity 383 382 -1
============================================
Files 68 68
Lines 2084 2079 -5
Branches 163 163
============================================
- Hits 1422 1419 -3
+ Misses 571 570 -1
+ Partials 91 90 -1 |
nkukday
reviewed
Nov 18, 2019
| public void setBaseUrl(String eventsHost) { | ||
| if (isValidUrl(eventsHost)) { | ||
| public synchronized boolean setBaseUrl(String eventsHost) { | ||
| if (isValidUrl(eventsHost) && checkNetworkAndParameters()) { |
There was a problem hiding this comment.
do you need to check for network here?
There was a problem hiding this comment.
TelemetryClient is created inside that check. It's a pipeline used by MapboxTelemetry class to init TelemetryClient
There was a problem hiding this comment.
checkNetworkAndParameters() checks for both network connection and init as required, so if you need the network check then this is fine, if the intent is to check and init TelemetryClient then you could checkRequiredParameters(..)
There was a problem hiding this comment.
we discussed it internally and decided to check network connection as well.
nkukday
approved these changes
Nov 20, 2019
|
Approved, could you rebase and merge? Thanks! |
korshaknn
force-pushed
the
fix-crash-on-telemetry-client
branch
from
01f58c9
to
e977ddb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR fixes issue 424