nknize commented on issue #608: LUCENE-8620: LatLonShape contains URL: https://github.com/apache/lucene-solr/pull/608#issuecomment-476690169 This is coming along nicely and I like the overall approach. A few general thoughts: 1. Would it make sense to split this PR into a couple separate issues? Maybe create a PR for the Tessellator logic that labels the Polygon edges along with the changes to `LatLonShape.encodeTriangle`? I think that will help make the changes more incremental and make it easier to review? 2. I'd also like to make sure we get the `componentRelateTriangle` and `componentRelate` logic tight in [LUCENE-8736](https://issues.apache.org/jira/browse/LUCENE-8736) first, and probably the Polygon quantization issues corrected, before fully merging this PR. IMHO it makes sense to ensure the polygon relation logic is fully sound before adding all of the `CONTAINS` changes?
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
