-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[#21515][Go SDK] Update go protobuf package to new version #32045
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, for create_test.go
the code there is validating that a ProtoV1 message is supported and can be encoded.
You can remove the marshal interface lines.
_ proto.Marshaler = &testProto{}
_ proto.Unmarshaler = &testProto{}
Since we aren't relying on the protoV1 unmarshalling handlers after this PR.
retest this please |
Thank you!! I had no idea about protoadapt package. I was trying to modify create_test.go to somehow work with proto v2 message. This helped clarified the goal of the test case.
Done. |
@lostluck |
It would go under 2.59.0 as 2.58 has already been cut and is going to be released next week. I agree that the change is worth a small note, in the unlikely event that it breaks a protov1 user. |
Thanks. I pushed the change. |
Assigning reviewers. If you would like to opt out of this review, comment R: @tvalentyn for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #32045 +/- ##
============================================
+ Coverage 57.18% 57.19% +0.01%
Complexity 1474 1474
============================================
Files 963 963
Lines 152697 152818 +121
Branches 1076 1076
============================================
+ Hits 87313 87409 +96
- Misses 63202 63220 +18
- Partials 2182 2189 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you very much! This is very helpful.
Thank you!! |
Update go protobuf package to new version.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.