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

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.

$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.

$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).

* @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