-
Notifications
You must be signed in to change notification settings - Fork 47
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
feat: Adding PolyUtil extensions to List<LatLng> #16
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 9 9
Lines 92 97 +5
Branches 18 18
======================================
- Misses 92 97 +5
Continue to review full report at Codecov.
|
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.
A few comments in-line.
Also, is Codecov not working? The bump in coverage in the Java lib of 20% when we moved tests from androidTest
to test
for Robolectric also seemed suspicious - googlemaps/android-maps-utils#649 (comment).
package com.google.maps.android.ktx | ||
|
||
import com.google.android.gms.maps.model.LatLng | ||
import com.google.maps.android.PolyUtil | ||
import com.google.maps.android.SphericalUtil | ||
|
||
/** | ||
* Computes where the given [latLng] is contained on or near this Polyline within a specified |
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.
where the given [latLng] is contained on or near
-> whether the given [latLng] lies on or near
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.
should this
be [this]
? I'm not sure how this
works with Kotlin docs.
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.
Just tested—[this]
doesn't work in Kotlin docs
inline fun List<LatLng>.isLocationOnPath( | ||
latLng: LatLng, | ||
geodesic: Boolean, | ||
tolerance: Double = 0.1 |
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.
0.1
should be DEFAULT_TOLERANCE
from the Java lib
inline fun List<LatLng>.isOnEdge( | ||
latLng: LatLng, | ||
geodesic: Boolean, | ||
tolerance: Double = 0.1 |
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.
0.1
should be DEFAULT_TOLERANCE
from the Java lib
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.
That's what I meant to do now that AMU 1.0.1 is live on maven. I'll need to bump the version used by the KTX lib and update this.
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.
Actually I'll address this as a separate PR (with version bumping AMU to 1.0.1). Will update the PR with doc edits though.
): Boolean = PolyUtil.isLocationOnPath(latLng, this, geodesic, tolerance) | ||
|
||
/** | ||
* Checks whether or not [latLng] lies on or is near the edge of this polygon within a tolerance |
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.
tolerance
is repeated here - maybe within the [tolerance] (in meters)
?
): Boolean = PolyUtil.isLocationOnEdge(latLng, this, geodesic, tolerance) | ||
|
||
/** | ||
* Computes whether the given point lies inside the specified polygon. |
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.
point
-> [latLng]
the specified
-> [this]
# 1.0.0 (2020-04-02) ### Bug Fixes * Add dependencies of maps-utils-ktx in pom. ([#14](#14)) ([d2f14e4](d2f14e4)) * next repositories inside publishing ([38e8dbc](38e8dbc)) * Use api instead of implementation. ([#10](#10)) ([ac40f46](ac40f46)) ### Features * Adding ability to publish to maven central ([#5](#5)) ([5cfa6b1](5cfa6b1)) * Adding KTX to Maps SDK ([#21](#21)) ([3313167](3313167)) * Adding PolyUtil extensions to List<LatLng> ([#16](#16)) ([d4cd6f5](d4cd6f5)) * Create maps-ktx module ([#20](#20)) ([597a70c](597a70c)) ### BREAKING CHANGES * Moved KTX for utils from com.google.maps.android.ktx to com.google.maps.android.ktx.utils to avoid package name conflicts with the new maps-ktx module. * refactor: move core extensions to separate package. * Using shared version. * Adjusting settings.gradle.
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Polygon.kt
already contain these extensions but these should be added toList<LatLng>
extensions as well.