Skip to content

Support proto3 field presence#79

Merged
wmatveyenko merged 1 commit intoarthurnn:mainfrom
sarahegler:sarah-proto3-field-presence
Apr 25, 2022
Merged

Support proto3 field presence#79
wmatveyenko merged 1 commit intoarthurnn:mainfrom
sarahegler:sarah-proto3-field-presence

Conversation

@sarahegler
Copy link
Contributor

@sarahegler sarahegler commented Apr 13, 2022 •
edited
Loading

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 sarahegler force-pushed the sarah-proto3-field-presence branch from 1a9c410 to 8a19f17 Compare April 20, 2022 19:26
@sarahegler
Copy link
Contributor Author

sarahegler commented Apr 20, 2022

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

@wmatveyenko
Copy link
Collaborator

wmatveyenko commented Apr 21, 2022 •
edited
Loading

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 •
edited
Loading

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

wmatveyenko commented Apr 25, 2022

Thanks again for your contribution!

@wmatveyenko wmatveyenko mentioned this pull request May 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sarahegler @wmatveyenko