Call Reset before UnmarshallVT to keep the default grpc semantic #131
Conversation
|
You should do the same trick as the code does to detect the |
|
In addition to @bhollis's comment I'd suggest to extract |
| @@ -25,6 +31,12 @@ func (Codec) Unmarshal(data []byte, v interface{}) error { | |||
| if !ok { | |||
| return fmt.Errorf("failed to unmarshal, message is %T (missing vtprotobuf helpers)", v) | |||
| } | |||
| //All types that implement github.com/golang/protobuf/proto.Message have a Reset method | |||
| vv, ok := v.(reseter) | |||
There was a problem hiding this comment.
| vv, ok := v.(reseter) | |
| vv, ok := v.(interface { ResetVT() }) |
There was a problem hiding this comment.
I can't require the message to implement ResetVT() because this method isn't generated by default (it requires the pool option)
Doing so would break the codec for user who just use the marshal and umarshal features
But I can follow this style and do
| vv, ok := v.(reseter) | |
| vv, ok := v.(interface{ Reset() }) |
There was a problem hiding this comment.
You're right :) IMO the resetvt func should be turned into a plug-in that's activated automatically with pool plugin.
There was a problem hiding this comment.
Don't you mean that ResetVT() should be generated automatically when the unmarshal plugin is enabled ?
Fixes #128
Contrary to what was discussed in the issue, I was not able to call
ResetVT()beforeUnmarshallVT()because the reset method isn't always generated (it's only generated when the pool option is enabled)Perhaps the
ResetVT()method should also be generated when the unmarshal feature is enabled ?