-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[3.0] Remove *.class parameters in bundles #12822
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
@stof can you please check this. (typos and posible mistakes) |
@mayeco Can you do the same for all bundles? |
</parameters> | ||
|
||
<services> | ||
<service id="validator" class="%validator.class%" factory-service="validator.builder" factory-method="getValidator" /> | ||
<service id="validator" class="Symfony\Component\Validator\ValidatorInterface" factory-service="validator.builder" factory-method="getValidator" /> |
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.
this is wrong currently, because the validator.class
parameter was replaced by the DI extension. You need to update the extension to set the service class instead now
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.
Thank you @stof, when you say "is wrong" I did something wrong? This PR is only for change *.class parameter to the actual class in the service configuration.
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.
Currently, the validator.class
parameter is modified in the container extension. You will have to change that to modify the service definition's class directly.
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.
done, I now change to the new class definition, confirm is ok please
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.
@mayeco you haven't changed the code in the DI extension to change the service class.
Even though the old API will be removed from 3.0, we should keep the code working in the meantime
@fabpot yes |
Done @fabpot. |
@fabpot What should I do with this tests?
|
@mayeco assert the service class instead |
@stof the Validator 2.4/2.5/2.5_BC workaround is going to be removed in 3.0, so that test should be remove also, right?. |
@mayeco only when the 2.4 API is removed. Until then, you need to keep it |
And as I commented in some diff which has been hidden, you need to update the FrameworkExtension so that it updates the service class directly instead of setting the parameter |
@stof for now lets keep the validator.class in this PR and I do other for the validator with the change in the parameter depending the api version, is that ok? |
@mayeco you can already remove the validator.class parameter in this PR (making the work of this PR complete). The only requirement is that you update other places setting this parameter, not only the XML file (and check for other parameters as well to see whether we have some code setting them elsewhere) |
Use the class parameter in the 'validator' service, because will change in runtime https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L755
@fabpot is done, but |
@mayeco As I said previously, you should change the FrameworkExtension to get the service definition and change its class directly. This will let you get rid of the parameter. the class is not changed at runtime. It is changed at configuration time. Decorators have nothing to do with this case. and can you rebase your PR to fix conflicts (the master branch has changed since last month) |
Rebasing is near to impossible, so I've created a new PR from scratch #14070 |
This PR was merged into the 3.0-dev branch. Discussion ---------- removed all *.class parameters | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | no | Fixed tickets | #11881, #12822 | License | MIT | Doc PR | none Commits ------- 8835d1a removed all *.class parameters
Remove the *.class parameters in all bundle settings.
see: #11881