Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Cookbook][Routing] Update custom_route_loader.rst#4790

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
weaverryan merged 4 commits intosymfony:2.3fromxelaris:custom_route_loader-tweaks
May 23, 2015

Conversation

xelaris
Copy link
Contributor

QA
Doc fix?yes
New docs?no
Applies to2.3+
Fixed tickets

Just some tweaks (including "Acme\DemoBundle" => "AppBundle").

While reading this chapter I wonder, if

A custom route loader allows you to add routes to an application without
including them, for example, in a YAML file. [...] This may be especially important when you want
to make the bundle reusable, or when you have open-sourced it as this would
slow down the installation process and make it error-prone.

may raise false expectations. Since also with a route loader it is still required to add something to the routing.yml (or xml). I don't see the benefit of using a custom route loader in a reusable bundle over importing a YAML file from a third-party bundle like FOSUserBundle. It makes totally sense for something like the FOSRestBundle does (generating routes based on conventions), but the chapter introduction sounds, like a Bundle could load its routes without manual tweaks to the config. If I am not wrong, this isn't possible, isn't it?

@wouterj
Copy link
Member

I agree with you, I see no benefit of a custom routing loader.

Some changes in this PR might conflict with#4740. Could you please check that and remove the ones that will conflict?

@xelaris
Copy link
ContributorAuthor

@wouterj The filecookbook/routing/custom_route_loader.rst is not part of#4740, isn't it?

@wouterj
Copy link
Member

Indeed, my bad. I thought it changed all cookbook articles.


.. note::

There are many bundles out there that use their own route loaders to
accomplish cases like those described above, for instance
`FOSRestBundle`_, `JMSI18nRoutingBundle`_, `KnpRadBundle`_ and `SonataAdminBundle`_.
`FOSRestBundle`_, `JMSI18nRoutingBundle`_, `KnpRadBundle`_ and
`SonataAdminBundle`_.
Copy link
Member

Choose a reason for hiding this comment

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

we might want to add AsseticBundle in the list actually

Copy link
Member

Choose a reason for hiding this comment

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

I would rather see a shorter list than making it longer

@weaverryan
Copy link
Member

@xelaris this looks great - nice work! But you're right - the first 2 paragraphs (especially the first) is terrible! I think we should mention FOSRestBundle as we do now, and make a new intro centered around it as one example.

Also, it's out of scope, but I think we should just show the class extendingLoader in the first example. That's much easier and more powerful. Then we can have a note that says that you can simply implementLoaderInterface instead of extendingLoader if you want.

@xelaris Can you at least take a shot at updating the first paragraph for us?

@xabbuh
Copy link
Member

ping@xelaris :)

@xelaris
Copy link
ContributorAuthor

Thank you for the ping@xabbuh. I will try to go on with this on the weekend.

@xelarisxelarisforce-pushed thecustom_route_loader-tweaks branch from6452b3f to9ec72b1CompareMarch 29, 2015 21:05
@xelaris
Copy link
ContributorAuthor

@weaverryan Thank you for the feedback. I have changed the first two paragraphs and tried to make it more clear.
In addition I have changed the code example to extend fromLoader as you suggested.

@xelaris
Copy link
ContributorAuthor

ping@weaverryan

have to create an ``extraAction`` method in the ``ExtraController``
of the ``AppBundle``::

namespace AppBundle\Controller;
Copy link
Member

Choose a reason for hiding this comment

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

you should add the filename comment above this:

// src/AppBundle/Controller/ExtraController.php

@xelarisxelarisforce-pushed thecustom_route_loader-tweaks branch from9ec72b1 to08b1527CompareApril 29, 2015 18:53
@xelaris
Copy link
ContributorAuthor

Thanks@xabbuh. I have added filename comments to all the php code blocks.

@wouterj
Copy link
Member

I don't think anything is missing here. Flagged it with Finished :)

@xabbuh
Copy link
Member

I agree 👍

@weaverryan
Copy link
Member

This looks really awesome - I have no changes. Thanks so much Alexander!

@weaverryanweaverryan merged commit08b1527 intosymfony:2.3May 23, 2015
weaverryan added a commit that referenced this pull requestMay 23, 2015
…aris)This PR was merged into the 2.3 branch.Discussion----------[Cookbook][Routing] Update custom_route_loader.rst| Q             | A| ------------- | ---| Doc fix?      | yes| New docs?     | no| Applies to    | 2.3+| Fixed tickets |Just some tweaks (including "Acme\DemoBundle" => "AppBundle").While reading this chapter I wonder, if> A custom route loader allows you to add routes to an application without including them, for example, in a YAML file. [...] This may be especially important when you want to make the bundle reusable, or when you have open-sourced it as this would slow down the installation process and make it error-prone.may raise false expectations. Since also with a route loader it is still required to add something to the routing.yml (or xml). I don't see the benefit of using a custom route loader in a reusable bundle over importing a YAML file from a third-party bundle like FOSUserBundle. It makes totally sense for something like the FOSRestBundle does (generating routes based on conventions), but the chapter introduction sounds, like a Bundle could load its routes without manual tweaks to the config. If I am not wrong, this isn't possible, isn't it?Commits-------08b1527 Add filename comments to code blocksfe1a574 Change code example to extend from Loader classbd6e3f3 Rewrite first paragraph46d10a2 [Cookbook][Routing] Update custom_route_loader.rst
@xelarisxelaris deleted the custom_route_loader-tweaks branchOctober 31, 2022 12:17
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@xelaris@wouterj@weaverryan@xabbuh@stof

[8]ページ先頭

©2009-2025 Movatter.jp