Skip to content

Added unittests for InternalGeoCentroid#24176

Merged
martijnvg merged 1 commit intoelastic:masterfrom
martijnvg:InternalGeoCentroidTests
Apr 24, 2017
Merged

Added unittests for InternalGeoCentroid#24176
martijnvg merged 1 commit intoelastic:masterfrom
martijnvg:InternalGeoCentroidTests

Conversation

@martijnvg
Copy link
Copy Markdown
Member

Relates to #22278

@martijnvg martijnvg added :Analytics/Aggregations Aggregations review >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1 labels Apr 19, 2017
Copy link
Copy Markdown
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I left a note about why we endcode and decode the point we generate but maybe just add a comment to the code for future and then merge?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to encode to a long and decode back to a GeoPoint here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we can reuse the current one and just set lat/lon on that. Let me change that.

@martijnvg martijnvg force-pushed the InternalGeoCentroidTests branch from 615ea14 to dabbf5d Compare April 24, 2017 14:57
@martijnvg martijnvg merged commit dabbf5d into elastic:master Apr 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants