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

restructured examples, added cpp sdk #12

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

restructured examples, added cpp sdk #12

wants to merge 5 commits into from

Conversation

DevrathIyer
Copy link
Collaborator

No description provided.

@ygina
Copy link
Collaborator

ygina commented Aug 27, 2021

Thanks for the pull request! Two high-level comments:

  1. Is it necessary to track all these files, particularly in the Python SDK? If they need to be regenerated when the Python source changes, it would be best to not track them but include instructions on how to regenerate them. If they need to be tracked because that's how the package manager works, it would still be better to include documentation on how to update and re-publish the package if necessary.
  2. Regarding how the karl-sensor-sdk directory is structured, it is a bit odd that it is now the root for what seems to b a Rust project and a C++ project and maybe also a Python project. I'd expect it to look more like this:

karl-sensor-sdk/
--- rust/
--- --- src/
--- --- examples/
--- --- Cargo.toml
--- cpp/
--- --- examples/
--- --- common.cmake
--- python/

Does that make sense, or is there a reason you structured it this way? The only other type of thing I might expect in the karl-sensor-sdk directory is a script that regenerates the gRPC file each language when it is updated.

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.

2 participants