Skip to content
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

Merged
merged 2 commits into from Jun 2, 2020
Merged

Conversation

stanley-cheung
Copy link
Contributor

@stanley-cheung stanley-cheung commented May 29, 2020

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 using uint64.

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.

Copy link
Contributor

@srini100 srini100 left a 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.

@stanley-cheung
Copy link
Contributor Author

stanley-cheung commented May 29, 2020

@srini100 sure will do an import. Please let me know before the release branch is cut.

Copy link
Contributor

@jtattermusch jtattermusch left a 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.

@stanley-cheung
Copy link
Contributor Author

There's an ongoing internal protobuf CL to change the signature to uint64_t now. Monitoring.

@stanley-cheung
Copy link
Contributor Author

Internal Protobuf CL submitted. I am running a test grpc import with cherry-picking this PR...

@stanley-cheung
Copy link
Contributor Author

The test import cherry-picking this PR all passed internal presubmits. Merging this now.

@stanley-cheung stanley-cheung merged commit fc9e509 into grpc:master Jun 2, 2020
@stanley-cheung stanley-cheung deleted the roll-forward-22998 branch June 2, 2020 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants