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

Jump to bottom

Update addressbook.proto, tutorial code and documentation #565

Closed
jskeet opened this issue Jul 1, 2015 · 3 comments
Closed

Update addressbook.proto, tutorial code and documentation #565

jskeet opened this issue Jul 1, 2015 · 3 comments
Labels
c#

Comments

@jskeet
Copy link
Contributor

jskeet commented Jul 1, 2015

Currently addressbook.proto is proto2.
We should either update it to proto3, or create a new equivalent for proto3. This should follow the new repeated fields naming convention of using plurals.

The csharp_experimental branch will soon have a proto3 version which has just removed the proto2 aspects of the file - we should consider this carefully before merging into master.
@jskeet jskeet added the c# label Jul 1, 2015
jskeet added a commit to jskeet/protobuf that referenced this issue Jul 9, 2015
@jskeet
Regenerated code due to previous commit.
5350822 
Note that now we need a proto3 version of addressbook.proto. This may affect other platforms, and could do with an overhaul to follow proto3 conventions anyway (e.g. repeated field names). Will need to think about that carefully before merging into master. Raised issue protocolbuffers#565 for this.
@jskeet jskeet mentioned this issue Jul 16, 2015
Closed
@jtattermusch
Copy link
Contributor

@xfxyjwf @anandolee, should everyone be using the proto3 version of the file in the future?
If not, we can just add a proto3 specific version of addressbook.proto.

If we want everyone to migrate to proto3 addressbook (which shouldn't be hard, because the addressbook examples are pretty simple), we should probably do this work in upstream/master first and then pull this change into csharp-experimental rather than the other way round (we don't want too many unrelated changes in the big PR to merge C# to master, because it's going to be big enough).

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Jul 16, 2015

The examples directory is intended to show users how to build their project with protobuf (also to verify that protobuf installation works). We don't need to cover every API version there. As we are advocating proto3, changing addressbook.proto to proto3 seems good to me.

@jtattermusch jtattermusch mentioned this issue Jul 20, 2015
Merged
@jtattermusch
Copy link
Contributor

#628

taoso pushed a commit to taoso/protobuf that referenced this issue Aug 1, 2018
@dsnet
proto: do not allow unknown fields to satisfy required field bit (pro…
91cccdb 
…tocolbuffers#565)

A prior changed (see protocolbuffers#511) allows Unmarshal to treat fields with
unknown wire types with known tags as unknown fields.
When doing so, we must be careful not to set the required field bit.

For example, suppose we have:
	message Foo { require fixed32 my_field = 1; }

Then parsing a message with a field of type varint and tag 1 should
not satisfy the required that Foo.my_field be set.
adellahlou pushed a commit to adellahlou/protobuf that referenced this issue Apr 20, 2023
@dcodeIO
Added asJSON bytes as Buffer, see protocolbuffers#566; Fixed parsing …
9c9a66b 
…enum inner options, see protocolbuffers#565; Other minor optimizations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c#
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jskeet @xfxyjwf @jtattermusch