-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
94ae262
to
b6d3289
Compare
…k, removes necessity to add question mark to all mapboxMap invocations
b6d3289
to
4853b35
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.
This is great! The converter tool (and any tweaks you added) looks really reliable. I love how builders will be cleaned up in the Kotlin code :)
I added comments wherever I've seen obvious adjustments, we can continue with the rest as we go. Also, do you think we need annotations on private fields? Lint is not great at picking up when assigning anything but a raw null
to the field and it doesn't improve Kotlin interop by much, but adds a lot of lines of code and makes it less readable.
private Path path = new Path(); | ||
|
||
Bubble(RectF rect, ArrowDirection arrowDirection, float arrowWidth, float arrowHeight, float arrowPosition, | ||
Bubble(RectF rect, @NonNull ArrowDirection arrowDirection, float arrowWidth, float arrowHeight, float arrowPosition, |
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.
rect
should not be null
@@ -81,7 +84,7 @@ public int getIntrinsicHeight() { | |||
return (int) rect.height(); | |||
} | |||
|
|||
private void initPath(ArrowDirection arrowDirection, Path path, float strokeWidth) { | |||
private void initPath(ArrowDirection arrowDirection, @NonNull Path path, float strokeWidth) { |
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.
arrowDirection
should not be null
@@ -111,6 +112,7 @@ public ArrowDirection getArrowDirection() { | |||
* @param arrowDirection The direction of the arrow | |||
* @return this | |||
*/ | |||
@NonNull | |||
public BubbleLayout setArrowDirection(ArrowDirection arrowDirection) { |
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.
arrowDirection
should not be null
@@ -53,11 +55,11 @@ | |||
initialize(view, mapboxMap); | |||
} | |||
|
|||
InfoWindow(View view, MapboxMap mapboxMap) { | |||
InfoWindow(@NonNull View view, MapboxMap mapboxMap) { |
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.
mapboxMap
should not be null
initialize(view, mapboxMap); | ||
} | ||
|
||
private void initialize(View view, MapboxMap mapboxMap) { | ||
private void initialize(@NonNull View view, MapboxMap mapboxMap) { |
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.
mapboxMap
should not be null
@@ -540,7 +555,7 @@ private void resolveForMarker(MarkerHit markerHit, Marker marker) { | |||
hitTestMarker(markerHit, marker, hitRectMarker); | |||
} | |||
|
|||
private void hitTestMarker(MarkerHit markerHit, Marker marker, RectF hitRectMarker) { | |||
private void hitTestMarker(MarkerHit markerHit, @NonNull Marker marker, RectF hitRectMarker) { |
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.
hitRectMarker
should not be null
@@ -112,8 +117,8 @@ public void run() { | |||
} | |||
|
|||
@UiThread | |||
final void easeCamera(MapboxMap mapboxMap, CameraUpdate update, int durationMs, boolean easingInterpolator, | |||
final MapboxMap.CancelableCallback callback, boolean isDismissable) { | |||
final void easeCamera(@NonNull MapboxMap mapboxMap, CameraUpdate update, int durationMs, boolean easingInterpolator, |
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.
update
should not be null
@@ -118,7 +122,7 @@ void requestRender() { | |||
/** | |||
* May be called from any thread | |||
*/ | |||
void queueEvent(Runnable runnable) { | |||
void queueEvent(@Nullable Runnable runnable) { |
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.
runnable
should not be null
private void drawAttribution(MapSnapshot mapSnapshot, Canvas canvas, | ||
AttributionMeasure measure, AttributionLayout layout) { | ||
private void drawAttribution(@NonNull MapSnapshot mapSnapshot, @NonNull Canvas canvas, | ||
@NonNull AttributionMeasure measure, AttributionLayout layout) { |
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.
layout
should not be null
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.
Also seems like #11886 (comment) has not been fixed.
4853b35
to
5ec60dc
Compare
@LukasPaczos comments have been adressed, except #13071 (review). It seems to be the same as the proposed change in #11886 (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.
@tobrun that's right, seems like it has been handled with a different PR 🚀
Closes #11886 , This PR improves consuming our library from kotlin by applying as much as possible
@Nullable/@NonNull
annotations. Most changes were performed automatically usingAnalyze > Infer Nullity...
in Android Studio.