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

Include the size of values (and tags) for extensions, even if the value is the default #8219

Merged
merged 1 commit into from
Feb 1, 2021

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Jan 20, 2021

Fixes #8218.

@jskeet
Copy link
Contributor Author

jskeet commented Jan 28, 2021

Ping @haberman @jtattermusch

Copy link
Contributor

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from the C# perspective.
Also LGTM from the protobuf perspective but I'd rather defer to @haberman 's opinion there.

@jtattermusch jtattermusch merged commit cea3653 into protocolbuffers:master Feb 1, 2021
@haberman
Copy link
Member

haberman commented Feb 1, 2021

Yes this LGTM. Extensions should always be serialized if present, and should never have the proto3 behavior of comparing with the default.

@jskeet jskeet deleted the fix-extension-set branch February 1, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExtensionSet is inconsistent for default values
4 participants