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-2069 - Fix proto generation #132

Merged
merged 4 commits into from
Aug 3, 2023

Conversation

stuqdog
Copy link
Member

@stuqdog stuqdog commented Aug 2, 2023

No description provided.

@stuqdog stuqdog requested a review from acmorrow August 2, 2023 14:49
@stuqdog stuqdog marked this pull request as ready for review August 2, 2023 14:49
@stuqdog stuqdog requested a review from a team as a code owner August 2, 2023 14:49
@stuqdog stuqdog requested review from maximpertsov and jckras and removed request for a team August 2, 2023 14:49
@@ -87,11 +87,6 @@ if (VIAMCPPSDK_USE_DYNAMIC_PROTOS)
config/buf.gen.remote.plugin.yaml.in
buf.gen.yaml
)
elseif ((NOT VIAMCPPSDK_OFFLINE_PROTO_GENERATION) AND (VIAMCPPSDK_GRPCXX_VERSION VERSION_GREATER_EQUAL 1.41.1))
Copy link
Member

Choose a reason for hiding this comment

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

I remember having to turn these flags on in the command line to avoid problems in github when testing the submitted code and they helped with debugging. Why are we getting rid of them?

Copy link
Member

@jckras jckras left a comment

Choose a reason for hiding this comment

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

Added a question for my own understanding, LGTM

Copy link
Member

@acmorrow acmorrow left a comment

Choose a reason for hiding this comment

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

LGTM per earlier discussion.

@stuqdog stuqdog merged commit c4f76ec into viamrobotics:main Aug 3, 2023
2 checks passed
@stuqdog stuqdog deleted the fix-proto-generation branch August 3, 2023 11:21
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.

3 participants