Skip to content

Linestring clipping bug#98

Merged
artemp merged 2 commits intomasterfrom
linestring-clipping-bug
Jul 30, 2019
Merged

Linestring clipping bug#98
artemp merged 2 commits intomasterfrom
linestring-clipping-bug

Conversation

@artemp
Copy link
Contributor

@artemp artemp commented Jul 2, 2019

No description provided.

@artemp artemp requested review from millzpaugh and springmeyer July 2, 2019 09:19
@artemp artemp requested a review from a team as a code owner July 2, 2019 09:19
@artemp artemp self-assigned this Jul 2, 2019
@springmeyer
Copy link
Contributor

@artemp do you know why the valid=false was there originally? Did we think it was needed for something?

@artemp
Copy link
Contributor Author

artemp commented Jul 11, 2019

@artemp do you know why the valid=false was there originally? Did we think it was needed for something?

No entirely sure ^. This looks like copy&paste mistake after refactoring. The intention always was to collect all valid geometries in clipped set and skip invalid ones. With that extra valid=false inside the loop the logic became : if last geometry is invalid all set is declared as invalid. Which doesn't make sense really. Let me know if this explains clearly enough. /cc @springmeyer

@springmeyer
Copy link
Contributor

This looks like copy&paste mistake after refactoring.

Okay. Looks like it landed in e589565

Let me know if this explains clearly enough

Yes, thanks for the more detailed explanation. Sounds good 👍

@springmeyer
Copy link
Contributor

codecov/project — 99.37% (-0.01%) compared to baf3b42

Note: there is a bug in codecov that we've reported to them (and they've confirmed) that makes PRs like this fail that remove code, when actually the coverage did not change. So, we can ignore this.

Copy link
Contributor

@springmeyer springmeyer left a comment

Choose a reason for hiding this comment

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

👍

@artemp artemp requested review from ian29 and removed request for millzpaugh July 12, 2019 09:05
@mapsam
Copy link
Contributor

mapsam commented Jul 16, 2019

Thanks @artemp!

@artemp artemp merged commit 02d352b into master Jul 30, 2019
@mapsam mapsam deleted the linestring-clipping-bug branch July 30, 2019 18:34
@millzpaugh millzpaugh mentioned this pull request Aug 14, 2019
@mapsam mapsam mentioned this pull request Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants