change block doc to best practices #794
Conversation
| @@ -56,7 +56,7 @@ for instance ``acme_main.block.rss``:: | |||
|
|
|||
| public function getType() | |||
| { | |||
| return 'acme_main.block.rss'; | |||
| return 'block.rss'; | |||
There was a problem hiding this comment.
It's often suggested to prefix with app. to avoid conflicts with a bundle called "block" for instance.
| @@ -125,7 +125,7 @@ that knows how to fetch the feed data of an ``RssBlock``:: | |||
| $resolver->setDefaults(array( | |||
| 'url' => false, | |||
| 'title' => 'Feed items', | |||
| 'template' => 'AcmeMainBundle:Block:rss.html.twig', | |||
| 'template' => 'AppBundle:Block:rss.html.twig', | |||
There was a problem hiding this comment.
app bundle templates should live in app/Resources/views/ and should be lowercased. So this would become block/rss.html.twig
| sandbox_main.block.rss: | ||
| class: Acme\MainBundle\Block\RssBlockService | ||
| # app/config/services.yml | ||
| block.rss: |
There was a problem hiding this comment.
app. prefix here as well (and I suggest also showing the services: key in this example)
| arguments: | ||
| - "acme_main.block.rss" | ||
| - "block.rss" | ||
| - "@templating" | ||
| tags: | ||
| - {name: "sonata.block"} |
There was a problem hiding this comment.
there should be spaces inside {} in Yaml
| - "@templating" | ||
| tags: | ||
| - {name: "sonata.block"} | ||
|
|
||
| .. code-block:: xml | ||
|
|
||
| <service id="sandbox_main.block.rss" class="Acme\MainBundle\Block\RssBlockService"> | ||
| <!-- app/config/services.xml --> | ||
| <service id="sandbox_main.block.rss" class="AppBundle\Block\RssBlockService"> |
There was a problem hiding this comment.
And actually, it's recommended to make services short and with only one dot. So app.rss_block
There was a problem hiding this comment.
i think this is a best practice i will ignore in my own projects. i find app.block.rss more logic. but for the cmf we should of course stick to those best practices as they are the compromise by the symfony community.
| <argument type="service" id="templating" /> | ||
| </service> | ||
|
|
||
| .. code-block:: php | ||
|
|
||
| // app/config/services.php | ||
|
|
There was a problem hiding this comment.
this empty line should be removed
| <argument type="service" id="templating" /> | ||
| </service> | ||
|
|
||
| .. code-block:: php | ||
|
|
||
| // app/config/services.php | ||
|
|
||
| use Symfony\Component\DependencyInjection\Definition; | ||
| use Symfony\Component\DependencyInjection\Reference; | ||
|
|
||
| $container | ||
| ->addDefinition('sandbox_main.block.rss', new Definition( |
|
thanks, updated |
|
i actually did it inconsistently on purpose, to distinguish between the block type and the service name. (i find the sonata block thing quite confusing and very complicated. so maybe i just got wrong what the type is supposed to do). but if you think its better this way, i am fine with it. |
i will follow up with further PRs for the other sections, but first want feedback on this one.
fixes part of #591