search

georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.49k stars 194 forks source link

refactor: replace LineOrPoint struct with enum #1047

Closed RobWalt closed 11 months ago

RobWalt commented 12 months ago

This small PR swaps out the LineOrPoint struct with an enum approach. It doesn't change the functionality of the type but the use of it should be much more readable now.

@rmanoka pinging you since it is mainly your code and I would appreciate a review if you find the time. I'm trying to make improvements step by step but I need to understand the code better (hence this PR). Are PRs like this ok for you?

rmanoka commented 12 months ago

lgtm too; can you fix the lint and fuzz CI errors?

RobWalt commented 12 months ago

Just looked into it. The error is

 --> geo/src/algorithm/sweep/line_or_point.rs:128:32
    |
128 |         T::Ker::orient2d(*self.left, *self.right, other)
    |                                ^^^^ method, not a field
    |

I just double checked and my code doesn't include this anymore. Could it be related to some kind of cache or dependency from geo to itself?

michaelkirk commented 11 months ago

CI updates your branch locally before running the tests to make sure they are compatible with what's in the main branch. Looks like the code in question was merged since your branch was created.

Can you either rebase and force push or merge main into your branch and address the issues?

RobWalt commented 11 months ago

Done :+1: