-
Notifications
You must be signed in to change notification settings - Fork 203
Fix EZP-24458: Refactor dynamic settings injection #1302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
178ad53
to
c7a00d6
Compare
// This one will be transformed | ||
$definition->addArgument( $dynamicSetting ); | ||
$resolverArguments[] = $this->createConfigResolverSubExpression( $this->parser->parseDynamicSetting( $dynamicSetting ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd wrap this.
It looks really good so far. Yay. |
c7a00d6
to
cefdcba
Compare
Review ping @andrerom @bdunogier @pspanja @glye @yannickroger |
It still looks rather good to me. |
330f2d6
to
8f231fa
Compare
+1, but as we need travis to run true tests please (cc @bdunogier) review the two PR's: |
8f231fa
to
f8554f1
Compare
Rebased against master to have Travis run properly again. |
OK tests are now passing \o/ |
Looks good, +1 |
@@ -41,7 +42,7 @@ public function process( ContainerBuilder $container ) | |||
{ | |||
$dynamicSettingsServices = array(); | |||
$resettableServices = array(); | |||
$fakeServices = array(); | |||
$updateableServices = $container->getParameter( 'ezpublish.config_resolver.updeatable_services' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"updeatable" speling? Not sure if existing bug or introduced here.
+1 apart from nitpick above. |
+1 |
b1063bd
to
b69e85c
Compare
@@ -21,7 +21,8 @@ class ConfigResolverParameterPassTest extends PHPUnit_Framework_TestCase | |||
public function testProcess() | |||
{ | |||
$container = new ContainerBuilder(); | |||
$dynamicSettingsServices = array(); | |||
$container->setParameter( 'ezpublish.config_resolver.updeatable_services', array() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last updeatable :)
b69e85c
to
e0568a1
Compare
> https://jira.ez.no/browse/EZP-24458 This PR removes usage of *fake* services to inject dynamic settings using factories. Instead, usage of ExpressionLanguage component is introduced.
e0568a1
to
6e9cccc
Compare
…ings Fix EZP-24458: Refactor dynamic settings injection
This PR removes usage of fake services to inject dynamic settings using factories.
Instead, usage of ExpressionLanguage component is introduced.
This allows to remove usage of a hack (fake services) and of a Symfony deprecated feature (synchronized services).
TODO