Allow loading different YAML extensions #1078
Conversation
| ]), new IdenticalPropertyNamingStrategy(), null, $this->getExpressionEvaluator()); | ||
| ]; | ||
|
|
||
| if ($addUnderscoreDir) { |
There was a problem hiding this comment.
This one broke getAllClassNames testm since this is invalid dir for most cases, but only getAllClassNames iterates over all directories and fails. So I made this optional
| @@ -0,0 +1,4 @@ | |||
| JMS\Serializer\Tests\Fixtures\Person: | |||
There was a problem hiding this comment.
Just copy-pasted some files in order to check loadability
There was a problem hiding this comment.
Hi,
thanks a lot for your PR and I'm sorry if it took so long to review it.
Beside two small comments to me looks good.
#1078 (comment) is about keeping maximum compatibility
#1078 (comment) is just a minor CS issue
src/Metadata/Driver/YamlDriver.php
Outdated
| use Symfony\Component\Yaml\Yaml; | ||
|
|
||
| class YamlDriver extends AbstractFileDriver | ||
| class YamlDriver implements AdvancedDriverInterface |
There was a problem hiding this comment.
Does it make sense just for compatibility to still extend AbstractFileDriver?
There was a problem hiding this comment.
All of it public and protected contracts were kept. Only instance of AbstractFileDriver should break. if it's not enougth we can keep extension with everything overrided
src/Metadata/Driver/YamlDriver.php
Outdated
| @@ -32,27 +36,74 @@ class YamlDriver extends AbstractFileDriver | |||
| * @var PropertyNamingStrategyInterface | |||
| */ | |||
| private $namingStrategy; | |||
| /** | |||
| * @var FileLocatorInterface|FileLocator | |||
There was a problem hiding this comment.
Do we need the FileLocator typehint here?
There was a problem hiding this comment.
Not really, just copy-pasted from metadata. Will fix it
|
@goetas fixed both comments |
|
Thanks a lot! |
Collaspse
AbstractFileDriverintoYamlDriverto allow multiple extensions (yaml,yml) to processed. Updated some testsFixes #1077
cc. @goetas