Skip to content
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

Add test for cellsToLinkedMultipolygon leak #673

Merged

Conversation

isaacbrodsky
Copy link
Collaborator

@isaacbrodsky isaacbrodsky commented Sep 4, 2022

Adds a test #669.

@isaacbrodsky isaacbrodsky mentioned this pull request Sep 5, 2022
@isaacbrodsky isaacbrodsky force-pushed the cells-to-linked-multipolygon-leak branch from 3bda16a to 82bd644 Compare September 5, 2022 01:30
@coveralls
Copy link

coveralls commented Sep 5, 2022

Coverage Status

Coverage increased (+0.06%) to 99.03% when pulling fb96105 on isaacbrodsky:cells-to-linked-multipolygon-leak into 6931d36 on uber:master.

@isaacbrodsky isaacbrodsky marked this pull request as ready for review September 5, 2022 01:40
// We assume that the input is a single polygon with loops;
// if it has multiple polygons, don't touch it
if (root->next) {
return NORMALIZATION_ERR_MULTIPLE_POLYGONS;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we make new H3Error codes here? I'm not sure if it matters for the end user at this point, but this change drops information about the source of the error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could - that information was not exposed to the user before.

@isaacbrodsky isaacbrodsky merged commit befff57 into uber:master Sep 6, 2022
@isaacbrodsky isaacbrodsky deleted the cells-to-linked-multipolygon-leak branch September 6, 2022 16:57
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.

None yet

4 participants