Replace php-http/message-factory to psr/http-factory, HttpClientDiscovery to Psr18ClientDiscovery, MessageFactoryDiscovery to Psr17FactoryDiscovery and Http\Client\HttpClient to Psr\Http\Client\ClientInterface #269
Conversation
FaboPT
changed the title
…ctoryDiscovery to Psr17FactoryDiscovery and Http\Client\HttpClient to Psr\Http\Client\ClientInterface
FaboPT
changed the title
|
@barryvdh Can you give an estimate for review that, please ? |
barryvdh
left a comment
barryvdh
left a comment
There was a problem hiding this comment.
Not really sure how to handle this BC-wise. If people supply their own Request Factory, it will now Break. So that requires a new major version..
| $uri | ||
| ) { | ||
| $request = $this->requestFactory->createRequest($method, $uri, $headers, $body, $protocolVersion); | ||
| $request = $this->requestFactory->createRequest($method, $uri); |
There was a problem hiding this comment.
This is a breaking change. Can we not set the headers/body/protocol on the request before sending?
| array $headers = [], | ||
| $body = null, | ||
| $protocolVersion = '1.1' | ||
| $uri |
There was a problem hiding this comment.
This is also a breaking change
| private $requestFactory; | ||
|
|
||
| public function __construct($httpClient = null, RequestFactory $requestFactory = null) | ||
| public function __construct($httpClient = null, RequestFactoryInterface $requestFactory = null) |
There was a problem hiding this comment.
So these are different interfaces, which would require significant BC break.
|
@barryvdh So it sounds like a new major version is required? Is there any reason not to do one? I know these things are just interfaces so not like a security risk, but folks auditing my dependencies don't know that. It'd be good to get this fixed. |
|
Alternative in #282 |
3 participants
No description provided.