Add support for zombie options #284
Conversation
|
This fails for PHP 5.3. I'm not sure what to do with this. |
Nothing. The fixing commit is Behat/Behat@3f5c206 , but since MinkExtension tests are using stable version of Behat and not |
|
@aik099 Thanks for the clarification. |
|
@aik099 Anything else I can do to support this change? |
| @@ -44,6 +44,10 @@ public function configure(ArrayNodeDefinition $builder) | |||
| ->scalarNode('server_path')->defaultNull()->end() | |||
| ->scalarNode('threshold')->defaultValue(2000000)->end() | |||
| ->scalarNode('node_modules_path')->defaultValue('')->end() | |||
| ->arrayNode('options') | |||
| ->prototype('scalar')->end() | |||
| ->defaultValue(array()) | |||
There was a problem hiding this comment.
->defaultValue(array()) is useless. This is already the case for a prototyped array node
| @@ -44,6 +44,10 @@ public function configure(ArrayNodeDefinition $builder) | |||
| ->scalarNode('server_path')->defaultNull()->end() | |||
| ->scalarNode('threshold')->defaultValue(2000000)->end() | |||
| ->scalarNode('node_modules_path')->defaultValue('')->end() | |||
| ->arrayNode('options') | |||
| ->prototype('scalar')->end() | |||
There was a problem hiding this comment.
please use 4 space for the indentation
| @@ -44,6 +44,10 @@ public function configure(ArrayNodeDefinition $builder) | |||
| ->scalarNode('server_path')->defaultNull()->end() | |||
| ->scalarNode('threshold')->defaultValue(2000000)->end() | |||
| ->scalarNode('node_modules_path')->defaultValue('')->end() | |||
| ->arrayNode('options') | |||
There was a problem hiding this comment.
please add useAttributeAsKey('name'). The value of the argument is not really useful (as we don't support XML config files, and YAML can define maps natively), but the call itself is necessary to ensure that keys are not lost when merging 2 configs together (and Behat relies on merging config between the default profile and other profiles for insrtance). Without this call, merging treats the array as a sequence, and so reindexes everything with numeric keys.
There was a problem hiding this comment.
done, I hope I understood what you wanted
There was a problem hiding this comment.
I can see in the code that it has been changed.
->useAttributeAsKey('name') is on line 48
|
I'm not sure if we have any tests that confirm, that options from |
|
@aik099 I couldn't find any. The only thing I could find was https://github.com/Behat/MinkExtension/blob/master/spec/Behat/MinkExtension/ServiceContainer/Driver/ZombieFactorySpec.php which didn't seem very specific. If you can give me a hint on how to add specific tests there I would gladly try to add these. |
|
If there are none, then none needed. |
|
The minkphp/MinkZombieDriver#183 is merged now and (if this repo is using dev versions of all drivers) we can actually see if it works. |
|
Hi. Any updates on this PR? I just wanted to set a zombie option, and found that it was no way to do it. |
|
PR is ready for merge. I'm missing merge permissions on this repository unfortunately, so it's the @stof , who can do the merging part here. |
|
@stof please merge this. I am waiting for it. |
|
@everzet Could you or someone else please merge this? |
|
@robbieaverill Could you have a look at may be merging this? No one seems to have time for this PR. |
|
@TerjeBr I'm not a maintainer of this package, sorry! |
|
Any maintainer of this package seeing this? Is @stof the only one who can merge things? |
|
What will it take to get this merged? Has this project been abandoned? Seems like no project mantainer with merge rights is around? |
|
@TerjeBr with respect, good things take time in open source. If you need the change you can fork the repository :) |
|
@everzet, @stof, @Shashikant86, @robocoder Anyone that have merge permissions to this repo give this some attention please. |
|
Ping. Could we have some progress on this please? |
|
Hello? Has this project been abandoned or something? |
|
I guess too few members used zombie (even I), and so interested in this. |
|
@stof Could you or some one else please merge this PR? |
|
@everzet A friendly ping ... |
#283 Add support for zombie options