+
Skip to content

Conversation

dfellis
Copy link
Collaborator

@dfellis dfellis commented Jul 8, 2021

Only two functions implemented so far, but this demonstrates the potential pattern possible. This makes the sub-command names case-insensitive and forces sub-commands to be in explicit locations in the argv list. (Not just hardwired to argv[1] in case we ever need sub-commands of sub-commands).

Work to tackle:

  • All of the rest of the public H3 functions as sub-commands
  • KML output (and input?) where appropriate
  • GeoJSON output (and input?) where appropriate

These can be in follow-up PRs, I hope?

@coveralls
Copy link

coveralls commented Jul 8, 2021

Coverage Status

Coverage remained the same at 98.991% when pulling 8bc69a8 on dfellis:master into 5203711 on uber:master.

@ajfriend
Copy link
Collaborator

This looks great! One question though. Do we have a way to add tests for these commands?

@dfellis
Copy link
Collaborator Author

dfellis commented Jul 12, 2021

This looks great! One question though. Do we have a way to add tests for these commands?

It would be integration testing (from the perspective of CMake) that we need to write. I have used shellspec to do this relatively cleanly in other projects. Do we want to potentially use that here but have it actively needed only if you run something like make bdd so most users don't have to download that?

Thoughts @isaacbrodsky @ajfriend @nrabinowitz ? (In my own usage, I have the Makefile acquire the version of shellspec I want to use as a dependency for running the BDD tests so it could also be done without requiring changes to setup for github actions or in local development.)

@isaacbrodsky
Copy link
Collaborator

I don't have any concerns with using an additional tool for testing the CLI. Using shellspec seems reasonable.

@dfellis dfellis merged commit f084317 into uber:master Jul 16, 2021
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

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载