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

Support proto3 field presence #79

Merged
merged 1 commit into from
Apr 25, 2022

Conversation

sarahegler
Copy link
Contributor

@sarahegler sarahegler commented Apr 13, 2022

Since protobuf 3.14, theoptional keyword can be used to distinguish between an unset and default primitive value. However, this code generator does not currently support the optional keyword as referenced in the following issue: #71

This update is to support code generation for protobuf schemas containing the optional keyword. It mirrors this change in the corresponding go code generator here: twitchtv/twirp#332

This change required using the google.golang.org/protobuf module which has superseded the github.com/golang/protobuf module, hence many changes in vendor/.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sarahegler
Copy link
Contributor Author

@marioizquierdo can you please take a look at this when you have a chance?

@wmatveyenko
Copy link
Collaborator

wmatveyenko commented Apr 21, 2022

Hi. Thanks for your pull request. Can you document how you tested these changes? And thanks for removing the .idea files ;-)

@sarahegler
Copy link
Contributor Author

sarahegler commented Apr 21, 2022

Hi @wmatveyenko, thanks for taking a look! I tested this in a private repository that uses this code generator using the protoc plugin with the --ruby_out option.

When using the latest released version of twirp-ruby without this change, I got the error mentioned in the linked issue <yourProto>.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-twirp_ruby hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--twirp_ruby_out:

When using my branch with this change, there was no error and code generation worked as expected. There was only a small change with the move to google.golang.org/protobuf where Twirp::Service is now prefixed with ::, e.g. instead of class MyService < ::Twirp::Service this produced class MyService < ::Twirp::Service (which I see was added in f408d65).

@wmatveyenko wmatveyenko merged commit 843fc83 into arthurnn:main Apr 25, 2022
@wmatveyenko
Copy link
Collaborator

Thanks again for your contribution!

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.

None yet

2 participants