[php] Fix PHPDoc so that UnaryCall defines the proper return type#37563
[php] Fix PHPDoc so that UnaryCall defines the proper return type#37563sayuprc wants to merge 5 commits intogrpc:masterfrom
Conversation
e2a3756 to
06ec922
Compare
692e91e to
6e5aa44
Compare
bshaffer
left a comment
There was a problem hiding this comment.
This is great, but we also need some tests for the generated output
| " * @param array $$options call options\n" | ||
| " * @return $return_type_id$\n */\n" | ||
| "public function $name$(\\$input_type_id$ $$argument,\n" | ||
| " $$metadata = [], $$options = []) {\n"); |
There was a problem hiding this comment.
I don't like all the code duplication this presents - why not do something like this instead? Then you'd only have to change a single line of code
if (method->server_streaming()) {
vars["return_type_id"] = "\\Grpc\\ServerStreamingCall";
} else {
vars["return_type_id"] = "\\Grpc\\UnaryCall<" + $vars["input_type_id"] + ">";
}
out->Print(vars,
" * @param \\$input_type_id$ $$argument input argument\n"
" * @param array $$metadata metadata\n"
" * @param array $$options call options\n"
" * @return $return_type_id$\n */\n"
"public function $name$(\\$input_type_id$ $$argument,\n"
" $$metadata = [], $$options = []) {\n");|
Thank you for your review. I have one question, though—I wasn't sure where to place the test code. |
|
@sayuprc you just need to follow the instructions in |
|
@bshaffer |
|
This is looking great! I'm not sure what's up with that "NO CHECK IN GEN CODE", though. We will need to look into that. @stanley-cheung what do you think? |
|
I apologize for the delayed response. Regarding What should I do? |
|
This is simply amazing. Is there anything I can do to help getting this merged? |
|
Messaging here for visibility is very helpful @Nyholm ! It's just waiting on me or @stanley-cheung to approve and merge. |
|
Sorry, there's some clang format diff here, do you mind fixing that? Thanks You may be able to just run |
|
@stanley-cheung |
|
This has been merged. Thanks for the contributions. This will be part of the next 1.74.0 release which will happen in the next couple of weeks. |
| * Wait for the server to respond with data and a status. | ||
| * | ||
| * @return array [response data, status] | ||
| * @return array{0: T|null, 1: stdClass} [response data, status] |
There was a problem hiding this comment.
@sayuprc shouldn't stdClass be imported or used as \stdClass under a namespace?
There was a problem hiding this comment.
Thank you for your feedback. You are correct—I'll make the necessary changes and open a PR.
…pc#37563) Type information is added to the result of `UnaryCall::wait()` execution. resolve grpc#33431 <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> Closes grpc#37563 COPYBARA_INTEGRATE_REVIEW=grpc#37563 from sayuprc:typing-support-for-php 0f1e6b2 PiperOrigin-RevId: 775541985
This PR addresses the following review comment: #37563 (comment) Changed `stdClass` to `\stdClass` to explicitly reference the global namespace. <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> Closes #39996 COPYBARA_INTEGRATE_REVIEW=#39996 from sayuprc:use-global-namespace ea680d1 PiperOrigin-RevId: 775872442
…pc#37563) Type information is added to the result of `UnaryCall::wait()` execution. resolve grpc#33431 <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> Closes grpc#37563 COPYBARA_INTEGRATE_REVIEW=grpc#37563 from sayuprc:typing-support-for-php 0f1e6b2 PiperOrigin-RevId: 775541985
This PR addresses the following review comment: grpc#37563 (comment) Changed `stdClass` to `\stdClass` to explicitly reference the global namespace. <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> Closes grpc#39996 COPYBARA_INTEGRATE_REVIEW=grpc#39996 from sayuprc:use-global-namespace ea680d1 PiperOrigin-RevId: 775872442
Type information is added to the result of
UnaryCall::wait()execution.resolve #33431