Simplify extractor service configuration #396
Conversation
| tags: | ||
| - { name: 'php_translation.extractor', type: 'php' } | ||
| _defaults: | ||
| autowire: true |
There was a problem hiding this comment.
As discussed in #372 we should not use autowire.
There was a problem hiding this comment.
Will remove it from the defaults 👍
The bundle currently has 4 occurences of using autowiring, so moving it to the level of the service itself seems ok: https://github.com/php-translation/symfony-bundle/search?q=autowire%3A+true&unscoped_q=autowire%3A+true
| Translation\Extractor\Visitor\Php\Symfony\ContainerAwareTrans: | ||
| tags: | ||
| - { name: 'php_translation.visitor', type: 'php' } | ||
| Translation\Extractor\FileExtractor\PHPFileExtractor: |
There was a problem hiding this comment.
Could we / should we use the same logic for extractors or does it makes no sense?
There was a problem hiding this comment.
What logic are you referring to?
| { | ||
| $vendorReflection = new \ReflectionClass(FormTypeChoices::class); | ||
|
|
||
| return \dirname($vendorReflection->getFileName(), 4); |
There was a problem hiding this comment.
Won't it be problematic in some edge cases?
For example, if you use a deployment server to warmup the cache before deploying , as the dirname function use the absolute path, it will be problematic if your project is not on the same location on your deployment server & your production server.
There was a problem hiding this comment.
The reason why I used FormTypeChoices is because that class already was in code, and thereby already is loadable. If FormTypeChoices would not be supported anymore in the future, more functionality will break. As the extension only runs on compilation of the container, this will keep working.
Related to the dirname function; Wouldn't the composer autoload then also break? The warmup will lead to a compiled container, so the path only is used to locate the classes in a resource. Afterwards, on runtime the composer autoloader will take over again.
|
replaced by #395 |
see #395 (comment)
replaces #395