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

RSDK-9556 Forward declarations for grpc Client stuff #341

Merged
merged 26 commits into from
Dec 23, 2024

Conversation

lia-viam
Copy link
Contributor

this is based off the currently open misc-proto-update branch

@lia-viam lia-viam requested a review from a team as a code owner December 17, 2024 20:59
@lia-viam lia-viam requested review from njooma and stuqdog and removed request for a team December 17, 2024 20:59
@lia-viam lia-viam changed the title Draft: RSDK-9556 Forward declarations for grpc Client stuff RSDK-9556 Forward declarations for grpc Client stuff Dec 18, 2024
// Forward declaration file for grpc ClientContext and ClientReaderInterface<T>.
// This file provides includes for recent (>= 1.32.0) versions of grpc.

namespace grpc {
Copy link
Member

Choose a reason for hiding this comment

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

(q) why is the namespace grpc here but grpc_impl in the else case below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In recent versions of grpc the classes are defined in namespace grpc, but in the oldest two versions we support the class definitions are in namespace grpc_impl and typedef'd into namespace grpc, which is incompatible with forward declaring the classes in namespace grpc

@lia-viam lia-viam merged commit 80efe0d into viamrobotics:main Dec 23, 2024
4 checks passed
@lia-viam lia-viam deleted the client-fwd branch December 23, 2024 18:11
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