Remove Zend\Stdlib dependency

[snapshotpl]

...and simplify config implementation

[Ocramius]

@snapshotpl to also be removed from composer.json

[snapshotpl]

@Ocramius that funny because Stdlib doesn't exists in composer.json on master :)

[snapshotpl]

It's very strange for me why current tests on master pass. When I pull master branch, run composer update and run test I get:

PHP Fatal error:  Class 'Zend\Stdlib\ArrayUtils' not found in zend-servicemanager/src/Config.php on line 62

How it's possible that travis said it's ok? Is it cache?

[weierophinney]

This should likely be a combination of isset && is_array, as every single item is expected to be an array. I'll make that change on merge.

[weierophinney]

Oof, on review, I realized that this is a rather large BC break, and cannot be merged. Why? Because it changes the behavior.

In the current stable version, if values are present in the $config property already, then any values provided via the passed $config array to the constructor are merged with those values. With this patch, they replace them, which is not and never was the intention.

The appropriate way to get rid of zend-stdlib is as @Ocramius suggested elsewhere: copy the functionality of ArrayUtils::merge into a trait or directly into the Config implementation. If you would like to amend this patch or provide a new one to do so, I'll review that.

[snapshotpl]

Sounds bad. Implementation of ArrayUrils::merge is more complicated than just copy - it uses other classes/interfaces (eg https://github.com/zendframework/zend-stdlib/blob/master/src/ArrayUtils.php#L272). I think we should add stdlib to dependency, or make BC break as it's enought early to nobody start use v3 🙈 🙉 🙊
What are you think? Same with $allowedKeys

[snapshotpl]

@weierophinney BTW I think is bug - tests on master don't pass.

[Ocramius]

Only arrayutils::merge needs polyfilling, btw.
On Jan 14, 2016 12:10 AM, "Witold Wasiczko" notifications@github.com
wrote:

@weierophinney https://github.com/weierophinney BTW I think is bug -
tests on master don't pass.

—
Reply to this email directly or view it on GitHub
#68 (comment)
.

[mtymek]

@weierophinney @Ocramius what about extracting ArrayUtils as a separate micro-library, same as it was done with hydrators? This way we will avoid code duplication. Yes, one external dependency would be still there, but we won't load lots of unnecessary classes.

[Maks3w]

I think StdLib is enough small and don't need to be splitted.

[Ocramius]

Agreed, stdlib is small enough
On Jan 14, 2016 12:03 PM, "Maks3w" notifications@github.com wrote:

I think StdLib is enough small and don't need to be splitted.

—
Reply to this email directly or view it on GitHub
#68 (comment)
.

[mtymek]

It is small if you take number of classes, but it has many responsibilities:

• ArrayUtils
• StringUtils
• interfaces for request and response
• priority list, priority queue
• ...

[Ocramius]

Wondering if we can just c&p the merging code (as private api)
On Jan 14, 2016 12:12 PM, "Mateusz Tymek" notifications@github.com wrote:

It is small if you take number of classes, but it has many
responsibilities:

• ArrayUtils
• StringUtils
• interfaces for request and response
• priority list, priority queue
• ...

—
Reply to this email directly or view it on GitHub
#68 (comment)
.

[snapshotpl]

Ok, but what about this feature? https://github.com/zendframework/zend-stdlib/blob/master/src/ArrayUtils.php#L272 Without this it's still break

[bakura10]

Can't it be simplified based on the context? For instance for a lib contains only a specific use case just Optimize the arrayUtils by removing useless stuff?

Envoyé de mon iPhone

Le 14 janv. 2016 à 12:28, Witold Wasiczko notifications@github.com a écrit :

Ok, but what about this feature? https://github.com/zendframework/zend-stdlib/blob/master/src/ArrayUtils.php#L272 Without this it's still break

—
Reply to this email directly or view it on GitHub.

[snapshotpl]

Ok, I copy ArrayUtils::merge to Config. I worried about MergeReplaceKeyInterface and MergeRemoveKey, however class doesn't need to exists to chech instance, so it's working. I have write unit test and I have found some issue in my implementation. I fixed that, however current implementation works same and looks simpler so I bring it back

[weierophinney]

I was concerned about the MergeReplaceKeyInterface and MergeRemoveKey usage, but realized that these are:

• advanced features
• opt-in features

We can make a suggestion in the composer.json indicating that if those features are needed, the user must install zend-stdlib.

Will merge shortly!

[Ocramius]

Note that the breakage still needs documenting, but it is indeed part of
protected API, so not a very huge deal

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 19 January 2016 at 22:07, weierophinney notifications@github.com wrote:

I was concerned about the MergeReplaceKeyInterface and MergeRemoveKey
usage, but realized that these are:

• advanced features
• opt-in features

We can make a suggestion in the composer.json indicating that if those
features are needed, the user must install zend-stdlib.

Will merge shortly!

—
Reply to this email directly or view it on GitHub
#68 (comment)
.

[weierophinney]

@Ocramius Documenting it in the CHANGELOG. I've also documented the features in the class-level docblock of the Config class, and added a suggest entry; you'll see this in the merge commit shortly.