Update to use PSR-11 for plugin managers #36
Conversation
- 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.
src/Reader/Json.php
Outdated
| */ | ||
| private function decode($data) | ||
| { | ||
| if (function_exists('json_decode')) { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
"require":{"ext-json":"*"} ;-)
There was a problem hiding this comment.
That's even easier... 😄
There was a problem hiding this comment.
Updated to remove zend-json entirely and require ext-json via Composer.
There was a problem hiding this comment.
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+)
There was a problem hiding this comment.
@webimpress seriously: their loss. A system without json support in 2017? :|
| 'php' => Writer\PhpArray::class, | ||
| 'phparray' => Writer\PhpArray::class, | ||
| 'xml' => Writer\Xml::class, | ||
| 'yaml' => Writer\Yaml::class, |
There was a problem hiding this comment.
Can we have here less spaces before =>, please?
src/Writer/Json.php
Outdated
| */ | ||
| public function processConfig(array $config) | ||
| { | ||
| return JsonFormat::encode($config); | ||
| if (function_exists('json_encode')) { |
There was a problem hiding this comment.
As noted above, probably we don't need it.
| } | ||
|
|
||
| /** | ||
| * @dataProvider supportedConfigExtensions |
There was a problem hiding this comment.
Can we add here also @param TYPE NAME for method parameters, please?
* @param string $extension
* @param string $expectedType| } | ||
|
|
||
| /** | ||
| * @dataProvider supportedConfigClassNames |
There was a problem hiding this comment.
Can we add here:
* @param string $classplease?
| } | ||
|
|
||
| /** | ||
| * @dataProvider supportedConfigTypes |
There was a problem hiding this comment.
As above, can we update PHPDocs here, please?
| } | ||
|
|
||
| /** | ||
| * @dataProvider supportedConfigClassNames |
src/Factory.php
Outdated
| { | ||
| static::$readers = $readers; | ||
| } | ||
|
|
||
| /** | ||
| * Get the reader plugin manager | ||
| * | ||
| * @return ReaderPluginManager | ||
| * @return ContainerInterface |
There was a problem hiding this comment.
Maybe we should have here also StandaloneReaderPluginManager, so generic one and specific/default?
Yes, I know that StandaloneReaderPluginManager implements ContainerInterface.
There was a problem hiding this comment.
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.
src/Factory.php
Outdated
| { | ||
| static::$writers = $writers; | ||
| } | ||
|
|
||
| /** | ||
| * Get the writer plugin manager | ||
| * | ||
| * @return WriterPluginManager | ||
| * @return ContainerInterface |
There was a problem hiding this comment.
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.
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
Factorywithout having zend-servicemanager installed.Additionally, this patch does the following: