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

Mumble.proto: explicitly use proto2 syntax. #2742

Merged
merged 1 commit into from Jan 20, 2017

Conversation

@mkrautz
Copy link
Member

commented Jan 20, 2017

No description provided.

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jan 20, 2017

The proto3 docs describe specifying proto3 syntax, and that proto2 will implicitly be used if not specified.

Is the use of syntax = "proto2"; documented somewhere? Or is this just our clarification and use of implied syntax?

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jan 20, 2017

@Kissaki Yes, I'll amend the commit with more info:

  • We're already using proto2
  • Why we need the syntax statement
  • Whether it's safe.

Why?
Protobuf 3.x.y's protoc gives a warning for proto files without an explicit syntax statement. (But it treats them as proto2).

Is it safe?
The syntax declaration is defined in the proto2 spec, so it should be safe.
https://developers.google.com/protocol-buffers/docs/reference/proto2-spec
(Search for "Syntax").

Mumble.proto: explicitly use proto2 syntax.
The protoc compiler in protobuf 3.x.y complains (warns) about
Mumble.proto not explicitly specifying a syntax. This commit
fixes that.

The syntax=XXX declaration is mentioned in language spec for
proto 2, so this should still work for protobuf 2.6, etc.
See https://developers.google.com/protocol-buffers/docs/reference/proto2-spec

@mkrautz mkrautz force-pushed the mkrautz:mumble-proto-proto2 branch from b686ec7 to 4e6d3bf Jan 20, 2017

@mkrautz mkrautz merged commit 4fe07a5 into mumble-voip:master Jan 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.