Allowing customization of host and port for ChromeManager #22
Conversation
src/ProcessManager/ChromeManager.php
Outdated
| { | ||
| $this->process = new Process([$chromeDriverBinary ?? $this->findChromeDriverBinary()], null, null, null, null); | ||
| $this->arguments = $arguments ?? $this->getDefaultArguments(); | ||
| $this->options = $options ?? $this->getDefaultOptions(); |
There was a problem hiding this comment.
Can you use array_merge to merge data by the user provided and default values (in case it's not provided). The default value can then be [].
There was a problem hiding this comment.
Yup! was just copying args
src/ProcessManager/ChromeManager.php
Outdated
| 'https' => false, | ||
| 'host' => '127.0.0.1', | ||
| 'port' => 9515, | ||
| 'status' => '/status' |
There was a problem hiding this comment.
'path' (again to be compatible with parse_url)?
src/ProcessManager/ChromeManager.php
Outdated
| private function getDefaultOptions(): array | ||
| { | ||
| return [ | ||
| 'https' => false, |
There was a problem hiding this comment.
'scheme' => 'https' to respect the format of parse_url and to allow using potential exotic schemes?
There was a problem hiding this comment.
Way better than what I had... :D
dunglas
left a comment
dunglas
left a comment
There was a problem hiding this comment.
Can you also add a mention of this new set of options in the README?
src/ProcessManager/ChromeManager.php
Outdated
| private $options; | ||
|
|
||
| public function __construct(?string $chromeDriverBinary = null, ?array $arguments = null) | ||
| public function __construct(?string $chromeDriverBinary = null, ?array $arguments = null, ?array $options = []) |
|
Not sure where this belongs in the readme. You have no examples of how to use the ChromeManager by itself. |
|
Please let me know where you'd like me to put this @dunglas |
|
Thanks! Regarding the documentation, what do you think of adding a new « Chrome » section in the README? |
* Allowing customization of host and port for ChromeManager * Update ChromeManager.php * Update ChromeManager.php * Update ChromeManager.php * Style fixes * Removing ?
* Allowing customization of host and port for ChromeManager * Update ChromeManager.php * Update ChromeManager.php * Update ChromeManager.php * Style fixes * Removing ?
2 participants
No description provided.