runner: replace deprecated jhump/protoreflect/desc in BinaryDataFunc - #468
Open
mkysel wants to merge 1 commit into
Open
runner: replace deprecated jhump/protoreflect/desc in BinaryDataFunc#468mkysel wants to merge 1 commit into
mkysel wants to merge 1 commit into
Conversation
The BinaryDataFunc type and WithBinaryDataFunc option exposed *desc.MethodDescriptor from github.com/jhump/protoreflect/desc in their public signatures. This package is deprecated in favour of the official google.golang.org/protobuf/reflect/protoreflect API. Callers who use WithBinaryDataFunc are forced to import the deprecated package solely to satisfy the function signature, even when they ignore the mtd parameter entirely. Replace *desc.MethodDescriptor with the standard protoreflect.MethodDescriptor interface at the public boundary. Internally, the existing *desc.MethodDescriptor is converted via its UnwrapMethod() accessor before being passed to the user-supplied function.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
BinaryDataFuncandWithBinaryDataFuncexpose*desc.MethodDescriptorfrom
github.com/jhump/protoreflect/descin their public API. That packageis deprecated upstream with the recommendation to migrate to
google.golang.org/protobuf/reflect/protoreflect.This forces callers to import the deprecated package just to satisfy the
function signature — even when the
mtdparameter is unused (which is thecommon case for binary data functions).
Solution
BinaryDataFuncto acceptprotoreflect.MethodDescriptor(thestandard interface from Google's v2 protobuf package) instead of
*desc.MethodDescriptordata.go, convert via(*desc.MethodDescriptor).UnwrapMethod()before passing to the user function
Breaking change
Any caller that currently references
*desc.MethodDescriptorin theirBinaryDataFuncwill need to update the parameter type toprotoreflect.MethodDescriptor. Sincegoogle.golang.org/protobufisalready a transitive dependency of ghz, no new dependency is introduced.