Use Psr7 consistently #239
Conversation
| @@ -353,7 +353,7 @@ public function handleResponse(ConnectionInterface $connection, ServerRequestInt | |||
| // response to HEAD and 1xx, 204 and 304 responses MUST NOT include a body | |||
| // exclude status 101 (Switching Protocols) here for Upgrade request handling below | |||
| if ($request->getMethod() === 'HEAD' || $code === 100 || ($code > 101 && $code < 200) || $code === 204 || $code === 304) { | |||
| $response = $response->withBody(Psr7Implementation\stream_for('')); | |||
| $response = $response->withBody(Psr7\stream_for('')); | |||
There was a problem hiding this comment.
To me, the Psr7 name implies this is the PSR-7 interface, whereas this is actually a concrete package that happens to implement this interface and in this particular case only references a function that resides in the same namespace. I'm all for consistency and I'm not opposed to improving things here, but what do you think about this?
There was a problem hiding this comment.
To me, the Psr7 name implies this is the PSR-7 interface,
Fair point. On the other hand side we don't care about what the actual implementation here is as long as there's any one. From that perspective and by concvention these two are interchangable for me.
What does imho not work is g7 vs. Psr7 vs. Psr7Implementation. Personally I prefer Psr7 as synonym which is also simple to use.
There was a problem hiding this comment.
This line of code does very much care about which implementation is being used here, as this function is not part of PSR-7. This function happens to be implemented by both guzzle/psr-7 and also ringcentral/psr-7, but there's no guarantee it's available for other implementations, because it's not actually part of PSR-7.
I agree that the existing code base is inconsistent here, but I think naming this just "Psr7" is actually misleading. What do you think about this?
There was a problem hiding this comment.
I think it's better than before and personally I prefer short names. Psr7Implementation is too long and Psr7 is imho a wide-spread, if not perfect, replacement. The fact that there may be multiple implementations shouldn't matter as a) we're only using one and b) it's entirely clear from the use statement which one that is. I really appreciate your eye on the details but would also feel it doesn't need to get academically correct?
|
@andig Is this still relevant with all the latest changes in master? |
|
Closing in order to drag the discussion on. I'd still be happy about more consistent psr7 naming ;) |
|
Lets discuss that after |
This is just a small code cleanup for sake of consistency across the various classes. No functional changes. Split from #235.