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

H3 CLI Region Subcommands #923

Merged
merged 16 commits into from
Oct 14, 2024
Merged

H3 CLI Region Subcommands #923

merged 16 commits into from
Oct 14, 2024

Conversation

dfellis
Copy link
Collaborator

@dfellis dfellis commented Oct 7, 2024

Still as a draft because I've only hand tested some simple cases, and I know there's an issue with reading large polygon definitions from a file, but I wanted to get what I have up sooner rather than later so you can start taking a look at it.

@coveralls
Copy link

coveralls commented Oct 7, 2024

Coverage Status

coverage: 98.782%. remained the same
when pulling 03b8c97 on h3-cli-region-subcommands
into e8b1661 on master.

src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
@dfellis dfellis marked this pull request as ready for review October 8, 2024 03:34
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
@dfellis
Copy link
Collaborator Author

dfellis commented Oct 8, 2024

I just ran this failure locally (set up CMAKE with the same configuration) and I can't reproduce it. @isaacbrodsky any idea what's going on here?

EDIT: Resolved it myself by blowing away my cmake cache with git -ffdx. I didn't actually run the tests with the configuration in that CI run when I wasn't reproducing it.

src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
dfellis and others added 4 commits October 13, 2024 15:21
Co-authored-by: Isaac Brodsky <isaac@isaacbrodsky.com>
Co-authored-by: Isaac Brodsky <isaac@isaacbrodsky.com>
src/apps/filters/h3.c Outdated Show resolved Hide resolved
Co-authored-by: Isaac Brodsky <isaac@isaacbrodsky.com>
Copy link
Collaborator

@nrabinowitz nrabinowitz left a comment

Choose a reason for hiding this comment

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

Left some comments, but overall this looks great!

src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Show resolved Hide resolved
src/apps/filters/h3.c Show resolved Hide resolved
src/apps/filters/h3.c Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Outdated Show resolved Hide resolved
src/apps/filters/h3.c Show resolved Hide resolved
@dfellis dfellis merged commit 33681ff into master Oct 14, 2024
38 checks passed
@dfellis dfellis deleted the h3-cli-region-subcommands branch October 14, 2024 20:52
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