Skip to content
This repository was archived by the owner on Sep 16, 2021. It is now read-only.

[routing] change doc to best practices #801

Closed
wants to merge 1 commit into from
Closed

Conversation

dbu
Copy link
Member

@dbu dbu commented Jan 25, 2017

No description provided.

@dbu dbu added the wip/poc label Jan 25, 2017
@@ -125,7 +125,7 @@ generic_controller

The controller to use when the route enhancers only determined the template but
no explicit controller. The value is the name of a controller using either the
``AcmeDemoBundle::Page::index`` or ``acme_demo.controller.page:indexAction``
``AppBundle::Page::index`` or ``app.page_controller:indexAction``
notation.
Copy link
Member

Choose a reason for hiding this comment

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

Well, actually, all controller formats are supported right? So maybe we shouldn't explicitely mention these 2 (as you can also do FQCN::methodName)

cmf_routing:
dynamic:
controllers_by_type:
editable: acme_main.controller:editableAction
editable: app.cms_controller:editableAction
Copy link
Member

Choose a reason for hiding this comment

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

As best practices don't suggest using controllers as services, what about using AppBundle:Cms:editable here?

<?xml version="1.0" encoding="UTF-8" ?>
<!-- app/config/config.xml -->
Copy link
Member

Choose a reason for hiding this comment

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

should be before the doctype

@@ -211,6 +214,7 @@ choose this controller to handle the request.
.. code-block:: xml

<?xml version="1.0" encoding="UTF-8" ?>
<!-- app/config/config.xml -->
Copy link
Member

Choose a reason for hiding this comment

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

same here

@@ -264,7 +272,7 @@ setting is set as controller.

<?xml version="1.0" encoding="UTF-8" ?>
<container xmlns="http://symfony.com/schema/dic/services">

<!-- app/config/config.xml -->
Copy link
Member

Choose a reason for hiding this comment

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

and here

@@ -61,6 +62,7 @@ is executed.
.. code-block:: xml

<?xml version="1.0" encoding="UTF-8" ?>
<!-- app/config/services.xml -->
Copy link
Member

Choose a reason for hiding this comment

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

... and here (you get the point, everywhere 😉 )

@dbu
Copy link
Member Author

dbu commented Jan 25, 2017

incorporated all this feedback in #804 that is against the correct branch

@dbu dbu closed this Jan 25, 2017
@dbu dbu removed the wip/poc label Jan 25, 2017
@dbu dbu deleted the best-practices-routing branch January 25, 2017 16:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants