Proper container configurator API #1891
Conversation
Wirone
changed the title
Wirone
force-pushed
the
proper-configurator-api
branch
from
32f9977 to
c992dde
Compare
|
@TomasVotruba any news on this? Is there any blocker that stop you from merging this? |
| return static function (\Rector\Core\DependencyInjection\Loader\Configurator\RectorContainerConfigurator $containerConfigurator): void { | ||
| }; | ||
|
|
||
| ?> |
There was a problem hiding this comment.
Could you add a test fixture to skip the other non-rector configs?
We don't want to override Symfony config/services.php to Rector ones :) also ECS should be skipped.
There was a problem hiding this comment.
How it should be checked? IMHO there's no need for such fixtures since this rule is optional (I did not add it to any rule set). People who want opt-in and use it should just configure paths to ignore for this rule.
There was a problem hiding this comment.
Probably e2e test, that single registered configured rule did some job.
| */ | ||
| final class RectorContainerConfigurator extends ContainerConfigurator | ||
| { | ||
| } |
There was a problem hiding this comment.
I could not find the test case, that would actually test this type in rector.php results in successfuly loaded Rector container.
Could you add one?
|
@Wirone I'm a bit ill 🤒 this week, so I only got to your PR. |
- Stable public API for configuring services - Rector rule for migrating Symfony configurator to Rector configurator
Wirone
force-pushed
the
proper-configurator-api
branch
from
c992dde to
e192094
Compare
|
I'll look into this one today 👍 |
As discussed in #7035 here's implementation of
RectorServiceConfiguratorand rector for migrating from Symfony's one.Rector rule
I've tested it on Rector's codebase and it works, but it's unnecessary to refactor internal code since
symfony/dependency-injectionis patched and locally has proper return type. Also I did not addUseRectorContainerConfiguratorRectorto any rule set because I think it should be opt-in for end users (explicit use in the config). But if you would like to add it, just let me know where it should be.