Closed ezimanyi closed 3 years ago
@pseudomuto : Pinging you here as suggested in the contributing docs. Thanks for this great project, and look forward to your comments on this PR!
+1 for this, using this project to help with my documentation and I love it. Want to try supporting experimental fields.
What is Changing?
Protobuf 3.12 includes experimental support for field presence via the
optional
keyword. Please see #422 for more details/discussion about field presence in proto3.This pull request updates
protoc-gen-doc
to support proto3 optional fields.How is it Changing?
This pull request:
template_test.go
including a test that will show the behavior changeprotoc-gen-go
to signal that it understandsproto3
optional fields (which means that compilation will no longer fail on such fields)optional
when encountering aproto3
optional
field.What Could Go Wrong?
The primary risk is that this change could inadvertently affect code generation for fields that are not
proto3
optional
fields. This could occur due to either:github.com/golang/protobuf
The test coverage for
proto2
field attributes seemed good, and the existing tests continue to pass after this change. I added some tests toproto3
fields as the first commit, so that I could ensure that these would not be affected by the change; these continue to pass after the later commits.I've also tested this manually on my project (which does not yet have
proto3
optional
fields), and found no diff in the generated markdown before/after. That being said, I think that the unit tests (which have a great variety of different field types) are a better indication of correctness here than the manual test I ran.Details of individual commits
Add tests on proto3 fields
The tests in template_test.go validate the template generated from proto2 fields but do not have any tests for proto3 fields.
Improve the test coverage by adding tests on proto3 fields before the upcoming commits that will slightly change the logic for proto3 fields.
Update github.com/golang/protobuf to v1.4.2
We need at least v1.4.1 to be able to implement proto3 optional semantics.
Add tests on proto3 optional fields
Let's add some tests on a message that contains proto3 optional fields.
As I did not want to affect the compilation of all the test protocol buffers with the --experimental_allow_proto3_optional flag, compile the one .proto file intended to test this path separately. Once the feature is no longer experimental (and behind a flag), this .proto can be compiled along with the other test fixtures.
Add a TODO on the single test that will be updated when we add support for proto3 optional fields in the next commit.
Implement optional semantics for proto3
This commit adds support for the proto3 optional keyword.
Proto3 optional fields will have the label "optional" added, to match the behaviour of proto2 optional fields.
Fields in proto3 that are not marked as optional are unaffected and will continue to have no label (unless they are repeated, in which case they will continue to have the "repeated" label).