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

Enable generic service generation in C++ #235

Conversation

gregmedd
Copy link
Contributor

@gregmedd gregmedd commented Aug 17, 2024

See https://protobuf.dev/reference/cpp/cpp-generated/#service

This is necessary for implementing L3 clients

@gregmedd gregmedd self-assigned this Aug 17, 2024
@gregmedd
Copy link
Contributor Author

@stevenhartley - should this target main, or do you want me to create a release / bugfix branch off of the alpha.3 tag?

@gregmedd gregmedd linked an issue Aug 17, 2024 that may be closed by this pull request
@stevenhartley
Copy link
Contributor

@stevenhartley - should this target main, or do you want me to create a release / bugfix branch off of the alpha.3 tag?

Not sure what you mean by this issue, all we need is in the generated protoc code.

@gregmedd
Copy link
Contributor Author

gregmedd commented Aug 17, 2024

Not sure what you mean by this issue, all we need is in the generated protoc code.

The generated C++ code from protoc does not include any of the service information by default. It must be enabled by adding the cc_generic_services = true setting to .proto files containing service descriptions.

The only alternative offered for C++ service code generation in the protobuf documentation is to implement a custom code generator plugin.

Since a change to the .proto files is required, and the uProtocol libraries are using 1.6.0-alpha3 right now, I wanted to know if we should branch off of the alpha3 tag to make this a small bug fix (maybe tagged 1.6.0-alpha3.1) or if this should be merged to main.

If this is merged to main, up-cpp would have to consume up-spec main until the next tagged release.

Copy link
Contributor

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

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

LGTM

@stevenhartley stevenhartley merged commit 2919c5b into eclipse-uprotocol:main Sep 19, 2024
2 checks passed
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.

USubscription Client: Implement and test
2 participants