Skip to content

spec: use official proto wrappers for primitive fields #131

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

Closed

Conversation

jdef
Copy link
Member

@jdef jdef commented Oct 30, 2017

@jdef
Copy link
Member Author

jdef commented Oct 30, 2017

I'm not very happy w/ the Makefile updates. I couldn't think of a better way to manage the path to wrappers.proto. It doesn't make sense to include the path to the golang version of wrappers in the official spec because the official spec should be language neutral. Looking at the standard version of wrappers.proto that ships w/ protoc, it's clear that the golang package for file is expected to live in the golang/protobuf tree. Furthermore, when NOT doing the mangle step the go get ... step fails because the official google/protobuf repo doesn't have any golang files in src/google/protobuf (where the upstream wrappers.proto lives).

Suggestions welcome

@jdef
Copy link
Member Author

jdef commented Oct 30, 2017

I should add, there's also the possibility of vendoring the wrappers.proto file that we're using, and then referring to the vendored copy. We're not vendoring anything yet, so I'm hesitant to open that can of worms simply for this change set.

@jdef
Copy link
Member Author

jdef commented Oct 30, 2017

The OOB feedback that I've received so far is that these changes are too aggressive: only the OPTIONAL fields should use the wrapper types.

@jdef
Copy link
Member Author

jdef commented Oct 31, 2017

Received additional OOB feedback that the consistency w/ respect to has_xxx funcs (in C++, for example) is good as-is.

@jdef jdef added this to the v0.1 milestone Oct 31, 2017
@jdef
Copy link
Member Author

jdef commented Nov 1, 2017

Discussed in the CSI WG meeting, determined that default values are "good enough" signals and that we simply need stronger language in the spec regarding the use of defaults to serve as signals for unspecified fields. The wrappers can be useful but are a bit unwieldy to deal with, depending on the programming language of the generated code.

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.

Wrapping OPTIONAL fields in proto3.
1 participant