Update to use PSR-11 for plugin managers by weierophinney · Pull Request #36 · zendframework/zend-config

weierophinney · 2017-02-16T20:33:54Z

This patch updates zend-config to typehint on PSR-11 for each of its plugin managers, and provides standalone versions that have no dependencies other than the PSR-11 interfaces; this allows usage of the Factory without having zend-servicemanager installed.

Additionally, this patch does the following:

Updates the minimum supported versions of dependencies to the latest releases of each. If both v2 and v3 versions exist, it allows either.
Updates the JSON reader and writer to allow usage with ext/json, falling back to zend-json if present. This allows usage without zend-json being present.
Adds malukehno/docheader as a development requirement, in order to enforce license headers in all class files.

- and conflict with container-interop < 1.2.0.

… plugin managers Allows usage with any PSR-11 implementation, not just the zend-servicemanager-based ones present.

Instead of zend-servicemanager variants. Ensures no requirement for zend-servicemanager for standalone usage of the `Factory`.

Set minimum supported versions to most recent releases of dependencies. In cases where 2.X and 3.X versions exist, and they are compatible, the most recent versons of each are `||`'d.

Will now default to ext/json when present.

- Now require `malukehno/docheader` in development. - Added configuration via `.docheader`. - `license-check` will check all `src/` files. - `license-check-all` will check all `src/` and `test/` files; there are test assets that are _expected_ to fail. - Travis runs `license-check` for targets doing CS checks.

michalbundyra · 2017-02-16T20:50:01Z

src/Reader/Json.php

+     */
+    private function decode($data)
+    {
+        if (function_exists('json_decode')) {


From PHP docs:

As of PHP 5.2.0, the JSON extension is bundled and compiled into PHP by default.
http://php.net/manual/en/json.installation.php

Yes, but many distributions still ship it as an optional package. As such, we still need to verify, and have zend-json as a polyfill.

"require":{"ext-json":"*"} ;-)

That's even easier... 😄

Updated to remove zend-json entirely and require ext-json via Composer.

Ok, I like it, but now we lost zend-json fallback IF somebody doesn't have this extension and can't add it (IF it is even possible to not have it now on PHP 5.6+)

@webimpress seriously: their loss. A system without json support in 2017? :|

michalbundyra · 2017-02-16T20:51:13Z

src/StandaloneWriterPluginManager.php

+        'php'            => Writer\PhpArray::class,
+        'phparray'       => Writer\PhpArray::class,
+        'xml'            => Writer\Xml::class,
+        'yaml'           => Writer\Yaml::class,


Can we have here less spaces before =>, please?

michalbundyra · 2017-02-16T20:51:55Z

src/Writer/Json.php

    public function processConfig(array $config)
    {
-        return JsonFormat::encode($config);
+        if (function_exists('json_encode')) {


As noted above, probably we don't need it.

michalbundyra · 2017-02-16T20:53:33Z

test/StandaloneReaderPluginManagerTest.php

+    }
+
+    /**
+     * @dataProvider supportedConfigExtensions


Can we add here also @param TYPE NAME for method parameters, please?

* @param string $extension * @param string $expectedType

michalbundyra · 2017-02-16T20:54:17Z

test/StandaloneReaderPluginManagerTest.php

+    }
+
+    /**
+     * @dataProvider supportedConfigClassNames


Can we add here:

* @param string $class

please?

michalbundyra · 2017-02-16T20:54:48Z

test/StandaloneWriterPluginManagerTest.php

+    }
+
+    /**
+     * @dataProvider supportedConfigTypes


As above, can we update PHPDocs here, please?

michalbundyra · 2017-02-16T20:55:04Z

test/StandaloneWriterPluginManagerTest.php

+    }
+
+    /**
+     * @dataProvider supportedConfigClassNames


Update PHPDocs please.

michalbundyra · 2017-02-16T20:56:21Z

src/Factory.php

     * Get the reader plugin manager
     *
-     * @return ReaderPluginManager
+     * @return ContainerInterface


Maybe we should have here also StandaloneReaderPluginManager, so generic one and specific/default?

Yes, I know that StandaloneReaderPluginManager implements ContainerInterface.

The whole point with this change was to signal that you can provide generic implementations based on ContainerInterface.

I'll update the method description to indicate the default implementation, however.

michalbundyra · 2017-02-16T20:57:03Z

src/Factory.php

     * Get the writer plugin manager
     *
-     * @return WriterPluginManager
+     * @return ContainerInterface


As above, maybe we need to have here ContainerInterface|StandaloneWriterPluginManager?

Instead of optionally requiring ext/json and/or zend-json, just require ext/json. This simplifies internal logic tremendously, and allows removal of a custom exception.

- Always install from lock file - If on 5.6, update the PHPUnit dependency to ensure we get a version that works with that PHP version. - If on the "latest" strategy, do a full update. - If on the "lowest" strategy, do a full update, with `--prefer-lowest`. - If test coverage is requested, install coverage utilities. - Use variables to detail both legacy and coverage dependencies.

Close #36

weierophinney added 9 commits February 16, 2017 12:07

Require psr/container

b1e66f0

- and conflict with container-interop < 1.2.0.

Update Factory to typehint on psr/container interface, not specific…

59921f5

… plugin managers Allows usage with any PSR-11 implementation, not just the zend-servicemanager-based ones present.

Implemented standalone reader plugin manager

a3daad2

Implemented standalone config writer plugin manager

5d842be

Register standalone plugin manager variants by default

a68e5ef

Instead of zend-servicemanager variants. Ensures no requirement for zend-servicemanager for standalone usage of the `Factory`.

Added migration chapter detailing standalone plugin managers

53ede3d

Updated to most recent releases of all dependencies

0b1e92b

Set minimum supported versions to most recent releases of dependencies. In cases where 2.X and 3.X versions exist, and they are compatible, the most recent versons of each are `||`'d.

Allow usage without zend-json

b67064f

Will now default to ext/json when present.

weierophinney added BC Break enhancement labels Feb 16, 2017

weierophinney added this to the 3.0.0 milestone Feb 16, 2017

Have PluginNotFoundException extend the component RuntimeException

625d316

michalbundyra reviewed Feb 16, 2017

View reviewed changes

weierophinney added 7 commits February 16, 2017 15:37

Removed redundant whitespace.

f40b06c

Added parameter annotations to tests with data providers

b20121c

Document defaults returned by get(Reader|Writer)PluginManager methods

8a23270

Require ext/json

1567bf0

Instead of optionally requiring ext/json and/or zend-json, just require ext/json. This simplifies internal logic tremendously, and allows removal of a custom exception.

Edited migration document slightly.

39d53c3

Added CHANGELOG entries for #36

f7dccff

weierophinney merged commit 9199d71 into zendframework:develop Feb 16, 2017

weierophinney added a commit that referenced this pull request Feb 16, 2017

Merge branch 'feature/psr-11' into develop

fba0377

Close #36

weierophinney deleted the feature/psr-11 branch February 16, 2017 22:21

weierophinney mentioned this pull request Feb 16, 2017

Update zend-i18n version requirement #10

Closed

Conversation

weierophinney commented Feb 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants