Skip to content

Conversation

@lia-viam
Copy link
Collaborator

@lia-viam lia-viam commented Jan 2, 2025

Some easy/minor changes to take google includes out of headers

@lia-viam lia-viam requested a review from stuqdog January 2, 2025 21:47
@lia-viam lia-viam requested a review from a team as a code owner January 2, 2025 21:47
@lia-viam lia-viam requested review from njooma and removed request for a team January 2, 2025 21:47
Comment on lines +11 to +12
// compiler. You may wish to comment/uncomment the define above as needed, or add the definition
// with `-D` to the compiler.
Copy link
Member

Choose a reason for hiding this comment

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

(q) just to confirm, is this a standard practice we'd expect people to be comfortable with/know how to do? Specifically, the idea (and syntax) of defining the variable for the compiler, and also the fact that we don't care how it's defined just so long as it is defined? I think the latter is probably pretty clear, but for the former I'd definitely be running to the internet to get more advice on how to proceed. That could just be my ignorance though!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so yes! This is CMake's version of Make/autoconf-generated config.h type headers, where a bunch of stuff will be #define'd or commented out.

The default case which I tried to expand on here is that we expect the variable to not be defined, because that corresponds to some pretty ancient grpc versions.

The other thing I tried to expand on was that this is a configured file which will then be installed with make install, so it's possible but a little weird that the user configures it with one version of grpc, does make install, and then later wants to use that installed version with a different grpc version, in which case they can manipulate the macro definitions with commetning/uncommenting or -D to make it work

@lia-viam lia-viam merged commit 0f636c5 into viamrobotics:main Jan 3, 2025
4 checks passed
@lia-viam lia-viam deleted the misc-insulate branch January 3, 2025 15:58
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