search

verloop / twirpy

Twirp's python implementation
The Unlicense
99 stars 20 forks source link

Support proto3 field presence #38

Closed sarahegler closed 2 years ago

sarahegler commented 2 years ago

Since protobuf 3.14, the optional keyword can be used to distinguish between an unset and default primitive value. However, this code generator does not currently support the optional keyword; code generation of files containing the optional keyword result in the following error:

example.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-twirpy hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.

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: https://github.com/twitchtv/twirp/pull/332

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

ofpiyush commented 2 years ago

Thanks for the PR!

Can you remove the .idea directory from the PR? I'll test this out over this weekend.

sarahegler commented 2 years ago

Thanks for looking @ofpiyush - did you have a chance to test this out?

ofpiyush commented 2 years ago

@sarahegler sorry no. My dev machine conked on Saturday, I set up a new one but forgot about it by the time I'd set new one up. 🙈

I'll check today.

ofpiyush commented 2 years ago

@sarahegler Thanks a ton for making this change! 🎉

@avinassh I've tested it on local, we can merge it.

I'll add an optional field to our example as well as a separate PR.

avinassh commented 2 years ago

Thank you @sarahegler for the PR and thank you @ofpiyush for testing it. I have merged it now 🥳