[Data Loader] Pass root paths to file system locator constructor (instead of loader) #963
Conversation
- Allow construction of Locator without data roots for BC - Mark FilesystemLocator::setOptions deprecated
robfrawley
added
the
Attn: Deprecation
|
I think this is ready to go. @cedricziel @antoligy Comments? |
Binary/Loader/FileSystemLoader.php
Outdated
| @@ -38,30 +38,48 @@ class FileSystemLoader implements LoaderInterface | |||
| /** | |||
| * @param MimeTypeGuesserInterface $mimeGuesser | |||
| * @param ExtensionGuesserInterface $extensionGuesser | |||
| * @param string[] $dataRoots | |||
| * @param LocatorInterface $locator | |||
| * @param string[]|LocatorInterface $locator | |||
| */ | |||
There was a problem hiding this comment.
I would also place an @deprecated doc here.
There was a problem hiding this comment.
Hmm; while I understand your intention, @deprecated is a doc-block scope declaration that would cause IDEs and other tools to believe the entire __construct() method had been deprecated, which is obviously not the case. Perhaps a comment explaining the depreciation would be more appropriate? We don't want people seeing "FileSystemLoader.php::__construct()" in their IDEs.
There was a problem hiding this comment.
Ahh, very good point and I didn't think of that. A better idea is adding a comment, or maybe an @see referencing this PR or the docs added in this PR?
There was a problem hiding this comment.
I think 23e79a7 should do the trick. :-)
Continuation of #937 to pass roots directly to
LocatorInterfaceinstead of toFileSystemLoader, in preparation for breaking changes in2.0.