-
Notifications
You must be signed in to change notification settings - Fork 120
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
removed AutoValue from geoJson #953
Conversation
d517acb
to
7bf6116
Compare
0be9ee7
to
e408e19
Compare
a65ac1e
to
17b3b63
Compare
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.
Great progress @osana!
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.
Looks good!
I had a bunch of nitpicks lined up like code styling or suppressing non-existent warnings, but those should get caught with an improved checkstyle and not block this PR. We should try and merge these changes as soon as possible so that we can fully test them as a part of the upcoming Maps SDK v7.3.0
release train.
protected List<Double> readPointList(JsonReader in) throws IOException { | ||
|
||
if (in.peek() == JsonToken.NULL) { | ||
throw new NullPointerException(); |
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.
Can we return null
here to print a more detailed from the calling method?
services-geojson/src/main/java/com/mapbox/geojson/BaseGeometryTypeAdapter.java
Show resolved
Hide resolved
this.boundingBoxAdapter = new BoundingBoxTypeAdapter(); | ||
} | ||
|
||
@SuppressWarnings("unchecked") |
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.
There's no warning to suppress.
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") |
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.
Same as above.
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") |
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.
Same as above.
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") |
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.
Same as above.
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") |
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.
Same as above.
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") |
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.
Same as above.
/** | ||
* Adapter to read and write coordinates for Point class. | ||
* | ||
* @since 4.1.0 |
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.
This should be 4.4.0
.
import java.util.List; | ||
|
||
/** | ||
* Adapter to read and write coordinates for Point class. |
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.
This should mention BoundingBox
class.
BoundingBoxDeserializer and BoundingBoxSerializer, GeometryDeserializer in favor of TypeAdapters
17b3b63
to
c293b78
Compare
address #710