add getter for response object #10
Conversation
so people may introduce their own assertions
|
I believe you should follow PSR-2 standard and use 4 spaces instead of tabs for indention. |
|
hi, thanks for the tip. changed all my pull requests to PSR-2 standards. |
|
What status of this ? Is there a another way to add custom check ? |
|
Hi Guys, still waiting for the pull request to be merged. what's the hold up? |
|
Can somebody merge this one please :) |
|
I'm also interested in this merge. I don't see a reason for the response attribute to be private, also. It should be protected, so child contexts can work with it. |
|
hey guys.. come on it's been months and we are only talking about a getter.. i mean, it's just one click away... what's the hold up? |
|
This would be really useful for me as well. |
|
This is pretty esential for writing custom assertions I think .. //cc @stof |
| @@ -59,6 +59,14 @@ public function setClient(Client $client) | |||
| } | |||
|
|
|||
| /** | |||
| * @return \GuzzleHttp\Message\ResponseInterface | |||
| */ | |||
| public function getResponse() | |||
There was a problem hiding this comment.
Should this be protected?
There was a problem hiding this comment.
why? everything else is public and i see no harm in that.
bests
There was a problem hiding this comment.
As far as I know it is not required outside of the scope of this class, in which case it is always better to encapsulate functionality.
There was a problem hiding this comment.
Maybe you could do the same of #27 here:
protected function getResponse()
{
if (! $this->response) {
throw new Exception("You must first make a request to check a response.");
}
return $this->response;
}I don't understand why this was not even reviewed, it has 23 comments and almost a year here.
This feature is very useful for custom contexts... Any way, I will probably copy WebApiContext and use it as my base context until this merge... 😢
|
Is there any reason why you don't wanna merged this one? |
|
Any news on this, @stof? |
|
Please merge 😄 |
|
It would be very useful for me too. Is there any reason to not merge it? |
|
The fact that it is public may be an issue since public visibility is probably not needed. But either way it needs to be accessible. I ended up writing my own. I should probably share it. |
|
+1 with |
|
Please merge 😄 [2] |
|
Jun 2014 and it's still not merged? Really? 👍 here....... |
|
This PR will make anniversary soon. 🎂 |
|
+1 for that! |
|
As a temporary solution you can always fork this project and fix it, then add the fork to your composer.json Simply remove the repository when this gets merged/closed to go back to the source. |
|
merge please |
|
🍰 Been over a year. Happy birthday. |
|
🍰 |
|
Happy birthday 🍰 |
|
Dear @stof , I consider it professional courtesy to at least reply to this thread. |
|
🍰 🎉 🎈 |
|
thx 🍰 |
|
IMO this PR should not approve, and we need to change this vars/methods to protected |
|
Changing the vars to protected would be ideal but there hasn't been ANY guidance given from the package maintainer. |
|
Happy birthday! 🍰 |
|
Hey guys, we are using this fork with Guzzle 6 support and protected client: https://github.com/LinioIT/behat-web-api-extension :) |
|
🍰 I believe getResponse() should be public so that assertions could be added by other Context classes without inheritance. Why isn't this PR merged yet? |
|
👍 Please make the response object available to child classes. Protected or getter; doesn't matter either way. I've had to copy/paste the entire |
|
Heres a +1 from 2016. Approaching a two year old PR! |
|
All these email notification about this issue just reminding me why I ditched PHP |
|
Should we organize a party on Jun 22? |
|
@AlmogBaku what because one library is being long about merging a pull request? if you want it that bad just fork it |
|
Since nobody cares i close this. It's been too long for just i minor merge. It really shows how to ignore the community. Sorry, but imho this really sucked. |
so people may introduce their own assertions