-
Notifications
You must be signed in to change notification settings - Fork 117
removed AutoValue from geoJson #953
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
Conversation
d517acb to
7bf6116
Compare
0be9ee7 to
e408e19
Compare
a65ac1e to
17b3b63
Compare
tobrun
left a comment
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