[FrameworkBundle] adds routing/container descriptors #7887
Conversation
|
@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. |
| } | ||
| } | ||
|
|
||
| $tags = $definition->getTags(); |
|
Will this be available in 2.4 ? |
| <default attribute="name">Joseph</default> | ||
| </defaults> | ||
| <requirements> | ||
| <requirement attribute="compiler_class">Symfony\Component\Routing\RouteCompiler</requirement> |
There was a problem hiding this comment.
the requirements for xml look wrong. these are options not requirements.
There was a problem hiding this comment.
ooops, thanks you! fixed now
| <method>GET</method> | ||
| <method>HEAD</method> | ||
| <defaults> | ||
| <default attribute="name">Joseph</default> |
There was a problem hiding this comment.
instead of "attribute" I would name this "name". so <default name="...">
There was a problem hiding this comment.
option tag already uses a name attribute, I guess it would be more consistent.
| @@ -0,0 +1,33 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <routes> | |||
| <route name="route_1" path="/hello/{name}" class="Symfony\Component\Routing\Route" path_regex="#^/hello(?:/(?P<name>[a-z]+))?$#s"> | |||
There was a problem hiding this comment.
it's missing the host regex
There was a problem hiding this comment.
I added it as attribute of the <host> tag, as the host regex existence depends on the host parameter existence.
There was a problem hiding this comment.
k, but then I would also make "path" an element. otherwise its pretty inconsistent.
There was a problem hiding this comment.
As the path is mandatory, I though it would good to keep it as attribute.
There was a problem hiding this comment.
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. see http://www.w3schools.com/dtd/dtd_el_vs_attr.asp
There was a problem hiding this comment.
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 layout bd16157 [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 controller d997dfa [FrameworkBundle] fixed container:debug and router:debug commands
This PR was merged into the master branch.
Discussion
----------
[FrameworkBundle] updated XML route description
This PR turns
```xml
<route name="route_1" path="/hello/{name}" class="Symfony\Component\Routing\Route" path_regex="#^/hello(?:/(?P<name>[a-z]+))?$#s">
```
into
```xml
<route name="route_1" class="Symfony\Component\Routing\Route">
<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/a
Commits
-------
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).