-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Centralize BoundingBox logic to a dedicated class #50253
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
Both geo_bounding_box query and geo_bounds aggregation have a very similar definition of a "bounding box". A lot of this logic (serialization, xcontent-parsing, etc) can be centralized instead of having separated efforts to do the same things
Pinging @elastic/es-analytics-geo (:Analytics/Geo) |
run elasticsearch-ci/1 |
aborted build for the upgrade, jenkins test this please |
run elasticsearch-ci/default-distro |
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.
Nice! I like the cleanup of GeoBoundingBoxQueryBuilder
. For posterity I'll note this also relates to the old old old issue: #10617
server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java
Outdated
Show resolved
Hide resolved
thanks for the review Nick, I've renamed the class! |
@elasticmachine update branch |
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.
LGTM
Both geo_bounding_box query and geo_bounds aggregation have a very similar definition of a "bounding box". A lot of this logic (serialization, xcontent-parsing, etc) can be centralized instead of having separated efforts to do the same things
Both geo_bounding_box query and geo_bounds aggregation have
a very similar definition of a "bounding box". A lot of this
logic (serialization, xcontent-parsing, etc) can be centralized
instead of having separated efforts to do the same things
relates to #50002 where yet another
bounds
context is to beintroduced in the geo*_grid aggregations.