Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Added the ability to set options for guzzle #965

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

polkfarody
Copy link
Contributor

No description provided.

Copy link
Contributor

@phansys phansys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status: Needs work.

*/
public function __construct(
$certificatePath = Resources::EMPTY_STRING,
$certificateAuthorityPath = Resources::EMPTY_STRING
$certificateAuthorityPath = Resources::EMPTY_STRING,
$options = array()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, declare the argument type.

@@ -127,6 +129,8 @@ public function __construct(

$this->_requestUrl = null;
$this->_expectedStatusCodes = [];

$this->_config = array_merge($this->_config, $options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the provided options be validated before hydrating $this->_config? (by instance, with an options resolver). BTW, if they are intended to be an associative array, + operator could be used instead of calling array_merge().

* @return IHttpClient
*/
protected function httpClient()
protected function httpClient($options = array())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this is a BC break (the method is extensible).

@@ -216,13 +218,13 @@ public function createTableService($connectionString)
*
* @return IServiceBus
*/
public function createServiceBusService($connectionString)
public function createServiceBusService($connectionString, $options = array())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this is a BC break (the method is extensible).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants