Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] adds routing/container descriptors#7887
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jfsimon commentedJun 4, 2013
@fabpot this PR is finally completed. Could you please check various output formats? They can be found in tests fixtures:https://github.com/jfsimon/symfony/tree/feat-framework-descriptors/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Duplicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yep, thanks.
deguif commentedSep 26, 2013
Will this be available in 2.4 ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the requirements for xml look wrong. these are options not requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ooops, thanks you! fixed now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
instead of "attribute" I would name this "name". so<default name="...">
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
option tag already uses aname attribute, I guess it would be more consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it's missing the host regex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I added it as attribute of the<host> tag, as the host regex existence depends on thehost parameter existence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
k, but then I would also make "path" an element. otherwise its pretty inconsistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As the path is mandatory, I though it would good to keep it as attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
attributes in xml are usually used for metadata (like the "class" that you have). but since the path is key info of a route it should be an element. seehttp://www.w3schools.com/dtd/dtd_el_vs_attr.asp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK thanks for the tip, I send a PR as soon as possible.
…bug commands (fabpot)This PR was merged into the master branch.Discussion----------[WIP] [FrameworkBundle] fixed container:debug and router:debug commands| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | n/a| License | MIT| Doc PR | n/a#7887 is buggy as hell :(Commits-------fe5961a [FrameworkBundle] moved router:debug and container:debug to use the compact layoutbd16157 [FrameworkBundle] changed JSON descriptors to be more readable on PHP 5.4+1d210f8 [FrameworkBundle] changed the router:debug to use the shortcut notation for the controllerd997dfa [FrameworkBundle] fixed container:debug and router:debug commands
This PR was merged into the master branch.Discussion----------[FrameworkBundle] updated XML route descriptionThis PR turns```xml<route name="route_1" path="/hello/{name}" path_regex="#^/hello(?:/(?P<name>[a-z]+))?$#s">```into```xml<route name="route_1"> <path regex="#^/hello(?:/(?P<name>[a-z]+))?$#s">/hello/{name}</path>```in XML routing description.| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | yes| Deprecations? | no| Tests pass? | yes| Fixed tickets |#7887| License | MIT| Doc PR | n/aCommits-------7b99ede [FrameworkBundle] updated XML route description
The goal of this PR is to add descriptors (as in#7454) for routing and container. This will permit add a
--formatoption torouter:debugandcontainer:debugcommands (withtxt,json,xmlandmdformats).