Skip to content

Fix const-ness of callback client read RPC requests#25763

Merged
vjpai merged 2 commits intogrpc:masterfrom
vjpai:cpp_gen
Mar 19, 2021
Merged

Fix const-ness of callback client read RPC requests#25763
vjpai merged 2 commits intogrpc:masterfrom
vjpai:cpp_gen

Conversation

@vjpai
Copy link
Copy Markdown
Contributor

@vjpai vjpai commented Mar 19, 2021

We don't use 1-sided streaming much, so I never noticed this obvious mistake. I have confirmed that the server side is ok.

@vjpai vjpai added lang/c++ release notes: no Indicates if PR should not be in release notes labels Mar 19, 2021
@vjpai vjpai requested a review from yashykt March 19, 2021 00:44
@vjpai vjpai enabled auto-merge (squash) March 19, 2021 00:45
@yashykt yashykt disabled auto-merge March 19, 2021 02:31
@yashykt
Copy link
Copy Markdown
Member

yashykt commented Mar 19, 2021

There seem to be a bunch of failing tests.

@vjpai
Copy link
Copy Markdown
Contributor Author

vjpai commented Mar 19, 2021

Yeah, the last round failed because I forgot to update the golden_file for the code generator. I think it's fixed now. The tests that have completed so far are passing, at least.

@vjpai vjpai enabled auto-merge (squash) March 19, 2021 02:56
@vjpai
Copy link
Copy Markdown
Contributor Author

vjpai commented Mar 19, 2021

Green now!

@vjpai vjpai merged commit 33d7aaf into grpc:master Mar 19, 2021
@yashykt
Copy link
Copy Markdown
Member

yashykt commented Mar 19, 2021

Does this need to be cherry-picked?

@vjpai vjpai deleted the cpp_gen branch March 19, 2021 08:14
@vjpai
Copy link
Copy Markdown
Contributor Author

vjpai commented Mar 19, 2021

Sure I'll send it through TGP just in case (it's unlikely that anyone is mocking this but you never know). I had set auto-merge on this so it's in, and I'm OOO on Friday but I can check in on it at some point.

@vjpai
Copy link
Copy Markdown
Contributor Author

vjpai commented Mar 19, 2021

@yashykt Thanks for the review, and good call. There is one mocker. I'll send a cherry-pick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang/c++ release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants