Update override.rst #4831
Update override.rst #4831
Conversation
ifdattic
commented
Jan 14, 2015
ifdattic
commented
| Q | A |
|---|---|
| Doc fix? | yes |
| New docs? | no |
| Applies to | 2.3 |
| Fixed tickets |
| Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3 | Fixed tickets |
|
👍 |
| @@ -68,7 +68,7 @@ in the core FrameworkBundle: | |||
| $container->setParameter('translator.class', 'Acme\HelloBundle\Translation\Translator'); | |||
|
|
|||
| Secondly, if the class is not available as a parameter, you want to make sure the | |||
| class is always overridden when your bundle is used, or you need to modify | |||
| class is always overridden when your bundle is used, or if you need to modify | |||
There was a problem hiding this comment.
could you please remove ther serial comma here?
| @@ -68,7 +68,7 @@ in the core FrameworkBundle: | |||
| $container->setParameter('translator.class', 'Acme\HelloBundle\Translation\Translator'); | |||
|
|
|||
| Secondly, if the class is not available as a parameter, you want to make sure the | |||
| class is always overridden when your bundle is used, or you need to modify | |||
| class is always overridden when your bundle is used or if you need to modify | |||
| something beyond just the class name, you should use a compiler pass:: | |||
There was a problem hiding this comment.
I think we should better reword this sentence completely. It reads really weird. What do you think?
There was a problem hiding this comment.
I think it wouldn't hurt to reword it, at least it should be split into two sentences to make it easier to read.
There was a problem hiding this comment.
Let's merge it this way. I'll create a new PR for 2.5, as this way of overriding services is deprecated anyway.
There was a problem hiding this comment.
@wouterj Actually, I think we can remove this from 2.3. This approach was considered "bad", but since there was no code change (e.g. in 2.5) that actually made it deprecated, I think we should show the "right" way in all versions. What do you think?
|
Consitency! Thanks @ifdattic :) |
