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

PHP: add call getTrailingMetadata API #6632

Merged

Conversation

stanley-cheung
Copy link
Contributor

@stanley-cheung stanley-cheung commented May 18, 2016

Now you can call $call->getTrailingMetadata() on all 4 types of calls we support.

Fixes #6580

@stanley-cheung
Copy link
Contributor Author

Friendly ping

@murgatroid99
Copy link
Member

I'm still not really sure why this is necessary. You already get the trailing metadata with the status.

@stanley-cheung
Copy link
Contributor Author

@jtattermusch want to chime in? I think the key usability issue here is that the $status object returned from a call wasn't well documented. It's not a well defined class. So it wasn't explicit to the user that you can do $status->metadata.

@jtattermusch
Copy link
Contributor

@stanley-cheung it seems like documentation is the key thing here. I think adding a new method to the call might make sense, but we need to properly document when the property can be accessed.
In any case, if $status is not properly documented, we should add the documentation for all the fields/methods users might care about.

@stanley-cheung
Copy link
Contributor Author

Fair enough. So we are OK with adding this getTrailingMetadata API, right? At least it will show up in the generated docs. But I also agree that we should document what fields/properties can be accessed in the $status object.

@stanley-cheung
Copy link
Contributor Author

@jtattermusch please confirm my previous comment?

@jtattermusch
Copy link
Contributor

@stanley-cheung I confirm.

@jtattermusch
Copy link
Contributor

will need a rebase to give jenkins a kick

@stanley-cheung stanley-cheung force-pushed the php-add-trailing-metadata-api branch from 26897a5 to 6668d51 Compare June 14, 2016 18:00
@stanley-cheung
Copy link
Contributor Author

Re-based and PHP tests passed

@jtattermusch jtattermusch merged commit 6f42fd9 into grpc:master Jun 15, 2016
@stanley-cheung stanley-cheung deleted the php-add-trailing-metadata-api branch June 15, 2016 03:02
@lock lock bot locked as resolved and limited conversation to collaborators Jan 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants