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
Add internal tags, remove Message from Call #223
Add internal tags, remove Message from Call #223
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.
This seems reasonable. What do you think about marking the GapicClientTrait
internal as well? That will leave us flexibility to loosen the types on signatures such as startCall
.
src/Call.php
Outdated
@@ -53,14 +51,14 @@ class Call | |||
/** | |||
* @param string $method | |||
* @param string $decodeType | |||
* @param Message $message | |||
* @param mixed $message |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
I don't think we can do that with GapicClientTrait. It is used in generated code. If we change it in a breaking way, then anyone who upgrades their gax version but doesn't upgrade their generated code would be broken. But if we want to change it in a non-breaking way, then we are fine to do that even if it isn't marked internal. Does that make sense? |
Codecov Report
@@ Coverage Diff @@
## master #223 +/- ##
=======================================
Coverage 86.99% 86.99%
=======================================
Files 68 68
Lines 2552 2552
=======================================
Hits 2220 2220
Misses 332 332
Continue to review full report at Codecov.
|
Yeah, it does - good call. |
Ready for review, PTAL |
WIP PR to add internal tags and remove references in Call to protobuf Message class, PTAL.