Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update override.rst #4831

Merged
merged 2 commits into from Jan 25, 2015
Merged

Update override.rst #4831

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc5564e
Update override.rst
ifdattic Jan 14, 2015
44b1584
Update override.rst
ifdattic Jan 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 14 additions & 7 deletions cookbook/bundles/override.rst
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ the routes from any bundle, then they must be manually imported from somewhere
in your application (e.g. ``app/config/routing.yml``).

The easiest way to "override" a bundle's routing is to never import it at
all. Instead of importing a third-party bundle's routing, simply copying
all. Instead of importing a third-party bundle's routing, simply copy
that routing file into your application, modify it, and import it instead.

Controllers
Expand Down Expand Up @@ -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::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should better reword this sentence completely. It reads really weird. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it wouldn't hurt to reword it, at least it should be split into two sentences to make it easier to read.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge it this way. I'll create a new PR for 2.5, as this way of overriding services is deprecated anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?


// src/Acme/DemoBundle/DependencyInjection/Compiler/OverrideServiceCompilerPass.php
Expand Down Expand Up @@ -106,7 +106,7 @@ Forms
-----

In order to override a form type, it has to be registered as a service (meaning
it is tagged as "form.type"). You can then override it as you would override any
it is tagged as ``form.type``). You can then override it as you would override any
service as explained in `Services & Configuration`_. This, of course, will only
work if the type is referred to by its alias rather than being instantiated,
e.g.::
Expand Down Expand Up @@ -136,7 +136,7 @@ the constraints to a new validation group:
.. code-block:: yaml

# src/Acme/UserBundle/Resources/config/validation.yml
Fos\UserBundle\Model\User:
FOS\UserBundle\Model\User:
properties:
plainPassword:
- NotBlank:
Expand All @@ -152,10 +152,17 @@ the constraints to a new validation group:
<?xml version="1.0" encoding="UTF-8" ?>
<constraint-mapping xmlns="http://symfony.com/schema/dic/constraint-mapping"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/constraint-mapping http://symfony.com/schema/dic/constraint-mapping/constraint-mapping-1.0.xsd">
xsi:schemaLocation="http://symfony.com/schema/dic/constraint-mapping
http://symfony.com/schema/dic/constraint-mapping/constraint-mapping-1.0.xsd">

<class name="FOS\UserBundle\Model\User">
<property name="plainPassword">
<constraint name="NotBlank">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changing it? I think the length is a great example, as NotBlank is already part of the FOS User afaik

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency because the YAML has two rules, the XML only one.
So in my opinion either XML should also have the notblank rule, or that rule should also be removed from YAML

<option name="groups">
<value>AcmeValidation</value>
</option>
</constraint>

<class name="Fos\UserBundle\Model\User">
<property name="password">
<constraint name="Length">
<option name="min">6</option>
<option name="minMessage">fos_user.password.short</option>
Expand Down