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
Roll forward #22998 #23093
Roll forward #22998 #23093
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.
LGTM but perhaps worth trying an import to make sure it won't get roll backed again. We are close to cutting a release branch.
@srini100 sure will do an import. Please let me know before the release branch is cut. |
47a7cf2
to
6f72faa
Compare
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 but we should verify this with an internal cherrypick before merging again.
There's an ongoing internal protobuf CL to change the signature to |
Internal Protobuf CL submitted. I am running a test grpc import with cherry-picking this PR... |
The test import cherry-picking this PR all passed internal presubmits. Merging this now. |
This is to roll forward #22998 after the rollback in #23089
We needed to roll #22998 back because of this import error: https://g3c.corp.google.com/results/invocations/c787c170-933c-4d84-88db-908739d593c1/targets/%2F%2Fgooglex%2Fdaedalus%2Fembedded%2Fbase%2Fnet%2Fngrpc2%2Fclient:channel_test/log
Apparently the protobuf spec told us to use
uint64_t
but internal is usinguint64
.I checked with the protobuf team and they said that
uint64_t
should be the correct one. Waiting for them to fix this on their side and then I will merge this again.This needs to go into the next 1.30 release.