feat: support google.golang.org/protobuf #119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
12 tasks
tac0turtle
reviewed
Mar 25, 2024
kocubinski
reviewed
Mar 25, 2024
| @@ -128,6 +131,19 @@ type JSONPBUnmarshaler interface { | |||
|
|
|||
| // Marshal marshals a protocol buffer into JSON. | |||
| func (m *Marshaler) Marshal(out io.Writer, pb proto.Message) error { | |||
| if msgV2, ok := pb.(protov2.Message); ok { | |||
There was a problem hiding this comment.
what is the thinking behind placing this v2 code path (and early return) before the nil and required field checks below which the v1 code path follows?
There was a problem hiding this comment.
That stuff except for maybe the nil check would be invalid for v2. Also this is for jsonpb so not performance critical - it's only used on the cli I believe.
kocubinski
approved these changes
Mar 29, 2024
Added |
julienrbrt
approved these changes
Apr 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for:
google.golang.org/protobuf/proto.MessagetypesMessageNameThis should remove any incompatibilties or need for type switching the SDK has when using these types.
Given that gogo proto already does a bunch of type switches to see what kind of message is used, this support seems reasonable and goes a long way to providing v2 support for anyone who wants it (we will use this at Regen at least and others will have a choice). Basically, this makes v2 types more with the v1 APIs the SDK uses without needing to change anything on the SDK side.