Skip to content

feat: accept proto field name when deserializing#72

Merged
kodiakhq[bot] merged 1 commit intoinfluxdata:mainfrom
rnarubin:proto_field_name
Oct 4, 2022
Merged

feat: accept proto field name when deserializing#72
kodiakhq[bot] merged 1 commit intoinfluxdata:mainfrom
rnarubin:proto_field_name

Conversation

@rnarubin
Copy link
Contributor

For message fields, the proto3 json specification requires parsers to accept both lowerCamelCase names and the original proto field names.

This change adapts the deserialization code-gen to accept the proto name.

Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you.

Could you please sign the CLA and I can get this in 😄

You could also rebase the commit message, to be semantic, but I am happy to do this for you

@tustvold tustvold changed the title Accept proto field name when deserializing feat: accept proto field name when deserializing Sep 30, 2022
@rnarubin
Copy link
Contributor Author

Alright. This is technically a corporate contribution, so I'll get in touch with my employer's legal team and get back to you with a ccla/icla

For message fields, the proto3 json specification requires parsers to
accept both lowerCamelCase names and the original proto field names.

This change adapts the deserialization code-gen to accept the proto
name.
@rnarubin
Copy link
Contributor Author

rnarubin commented Oct 3, 2022

Talked to legal, they cleared everything and I've signed the ICLA. I've also amended the commit to follow the semantic format

rnarubin added a commit to rnarubin/pbjson that referenced this pull request Oct 3, 2022
This change adds a flag to the code-gen builder, allowing output json
fields to use their proto-schema defined field names instead of being
converted to lowerCamelCase.

Preserving proto field names is mentioned as an optional feature in the
protobuf json specification[1]. This change builds on the previous
support[2] for deserializing preserved proto field names.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json_options
[2]: influxdata#72
@tustvold tustvold added the automerge Put in the merge queue label Oct 4, 2022
@kodiakhq kodiakhq bot merged commit 464915c into influxdata:main Oct 4, 2022
rnarubin added a commit to rnarubin/pbjson that referenced this pull request Oct 4, 2022
This change adds a flag to the code-gen builder, allowing output json
fields to use their proto-schema defined field names instead of being
converted to lowerCamelCase.

Preserving proto field names is mentioned as an optional feature in the
protobuf json specification[1]. This change builds on the previous
support[2] for deserializing preserved proto field names.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json_options
[2]: influxdata#72
rnarubin added a commit to rnarubin/pbjson that referenced this pull request Oct 18, 2022
This change adds a flag to the code-gen builder, allowing output json
fields to use their proto-schema defined field names instead of being
converted to lowerCamelCase.

Preserving proto field names is mentioned as an optional feature in the
protobuf json specification[1]. This change builds on the previous
support[2] for deserializing preserved proto field names.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json_options
[2]: influxdata#72
@tustvold tustvold mentioned this pull request Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Put in the merge queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants