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.

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

mention CMF ChainRouter, restructure documentation for extending routing #6020

Closed
wants to merge 1 commit into from
Closed

mention CMF ChainRouter, restructure documentation for extending routing #6020

wants to merge 1 commit into from

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Dec 16, 2015

Q A
Doc fix? no
New docs? yes
Applies to 2.3+
Fixed tickets Relates to #5709

redirect_trailing_slash
extra_information
extending_routing
custom_route_loader
multiple_routers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if its ok to add multiple levels of navigation here, but i think it would make sense. also not sure if this is the right syntax for rst.

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't work in the table of contents. You will probably want to update /cookbook/routing/map.rst.inc instead. But I am not sure if the symfony.com theme is able to render that properly (the platform.sh preview will tell us though).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no /cookbook/routing/map.rst.inc file i think. do you have an idea how we should do this? or should we give up on making this a sub-group and just cram it into one article? or a series that repeat part of the title: "Extending Routing: ..."?

Copy link
Member

Choose a reason for hiding this comment

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

Oh sorry about that. :( The right file is /cookbook/map.rst.inc.

Copy link
Contributor Author

@dbu dbu Dec 18, 2015 via email

Choose a reason for hiding this comment

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


The Symfony CMF ChainRouter allows to use more than one router. A main
use case is to keep the :doc:`default symfony routing system </book/routing>`
available when writing a custom controller.
Copy link
Member

Choose a reason for hiding this comment

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

custom router?

@wouterj
Copy link
Member

wouterj commented Dec 17, 2015

While I think it's good to write a short section about this, we have to make sure to not duplicate much from the CMF docs. The CMF Routing docs already suffer a lot from duplication (bundles, components & quick tour). Adding yet another one would make things hard to maintain.

@dbu
Copy link
Contributor Author

dbu commented Dec 18, 2015

agreed that we should not duplicate this here. the idea is to really just showcase what dynamic router does and then link to the cmf documentation.

@dbu
Copy link
Contributor Author

dbu commented Jan 4, 2016

any input on where to put the new sections in the navigation? should i put extending_routing, custom_route_loader and multiple_routers all into one page? or just list the 3 on one level? or add another level or hierarchy?

@dbu
Copy link
Contributor Author

dbu commented Feb 16, 2016

ping @weaverryan
would be glad to wrap this up. meanwhile this is in conflict with upstream - i will rebase and solve the conflicts once we decided how to handle this one. could you give some inputs on the questions above?

weaverryan added a commit that referenced this pull request Feb 16, 2016
This PR was merged into the 2.3 branch.

Discussion
----------

mention routing from the database

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | 2.3+
| Fixed tickets | #2186, relates to #5709

if we merge #6020, we might want to put this as subsection of the extending_routing section

Commits
-------

c458431 mention routing from the database
@xabbuh
Copy link
Member

xabbuh commented Feb 18, 2016

I think "extending routing" and "custom route loader" could share one document while "multiple routers" better fits in its own article.

process, e.g. JmsI18nRoutingBundle_.

.. _FrameworkExtraBundle: https://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/converters.html
.. _JMSI18nRoutingBundle: https://github.com/schmittjoh/JMSI18nRoutingBundle
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 link here: http://jmsyst.com/bundles/JMSI18nRoutingBundle - the GitHub page just points you there.

@weaverryan
Copy link
Member

Sorry for the delay @dbu! I think we should:

  • not do extending_routing at all - if we want, add more articles to the routing cookbook section to answer questions
  • keep "custom route loader" and the new "multiple routers" at the top level.

Overall, I really feel that having a lot of top-level "How do I do X" is useful.

Cheers!

@javiereguiluz
Copy link
Member

I'm closing this as "fixed" because:

xabbuh added a commit that referenced this pull request Mar 7, 2017
…uiluz)

This PR was merged into the 2.7 branch.

Discussion
----------

Added a mention to ChainRouter from Symfony CMF

This replaces #6020 to complete the mentions to Symfony CMF routers.

Commits
-------

63213a7 Added a mention to ChainRouter from Symfony CMF
@dbu dbu deleted the custom-router branch October 18, 2018 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants