This PR was squashed before being merged into the master branch (closes#5928).
Commits
-------
6a033f3 setData method also accepts objects. Doc should reflect this.
Discussion
----------
setData method also accepts objects. Doc should reflect this.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: NA
Todo: None
License of the code: MIT
Documentation PR: None
This PR was squashed before being merged into the master branch (closes#5904).
Commits
-------
84adcb1 [2.2][Routing] Added support for default attributes with default values of method params
Discussion
----------
[2.2][Routing] Added support for default attributes with default values of method params
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
With this patch, you can configure your default values likes this:
``` php
/**
* @Route("/hi/{name}", name="hi")
*/
public function hiAction($name = "Bob")
{
return new Response($name);
}
```
---------------------------------------------------------------------------
by Tobion at 2012-11-03T23:15:32Z
I'm unsure. How does one know if that param defines a default value or a requirement? It's too vague.
---------------------------------------------------------------------------
by lyrixx at 2012-11-03T23:35:27Z
It's only a default value, not a requirement.
It's just a shortcut to avoid `defaults={"name"="bob"}`
---------------------------------------------------------------------------
by Tobion at 2012-11-03T23:43:51Z
Yes, but its not clear. It could also be a shortcut to `requirements={"name"="bob"}`, which has totally different meaning. So it's not self-explanatory.
-1 for me.
---------------------------------------------------------------------------
by lyrixx at 2012-11-03T23:48:21Z
it is the default php behavior. It's a default value for a variable...
---------------------------------------------------------------------------
by stof at 2012-11-04T00:22:58Z
@Tobion using the default value of the method to set a requirement does not make any sense. I don't see why someone would expect this behavior
---------------------------------------------------------------------------
by fabpot at 2012-11-06T10:12:05Z
@lyrixx Can you add some unit tests?
---------------------------------------------------------------------------
by Tobion at 2012-11-06T10:28:42Z
Oh I misunderstood the PR. I thought this makes the `name` param default to `hi`. `@Route("/hi/{name}", name="hi")`. But it's just the name of the route. Your example was easy to misinterpret as you used `name` everywhere.
---------------------------------------------------------------------------
by fabpot at 2012-11-10T08:33:13Z
@lyrixx Can you finish this PR?
---------------------------------------------------------------------------
by lyrixx at 2012-11-10T13:16:34Z
@fabpot Yes i will as soon as possible.
---------------------------------------------------------------------------
by lyrixx at 2012-11-10T18:34:07Z
I rebase and amend my commit. (I changed doc in commit message to be less confusing)
I will try to add tests.
But for now, `AnnotationClassLoader::load` is not really tested, and `AnnotationClassLoader::addRoute` is absolutely not tested. So I think I should add tests for these methods ? And then add tests for my patch.
I will try tomorrow.
---------------------------------------------------------------------------
by lyrixx at 2012-11-11T18:23:41Z
@fabpot I added new tests. I tried to made very atomic commits.
This PR was merged into the master branch.
Commits
-------
bdf0334 Fixed the lap method. Added upgrade notes. Some CS fixes
Discussion
----------
Fixed the lap method. Added upgrade notes. Some CS fixes
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
License of the code: MIT
Documentation PR: ~
This adds some type-hinting to the Stopwatch components.
I've also split the Section class to its own file, I know it's not a must as per coding standards used by Symfony but it complies with most of the other classes in the framework.
I've updated the UPGRADE-2.2.md file as well.
There's a bug fix which I'm not sure it if should have been done in this branch or not.
Let me know if I should make this PR against an older version of the framework.
Thanks.
This PR was merged into the master branch.
Commits
-------
c7a8f7a [Routing] fixed possible parameters conflict in apache url matcher
Discussion
----------
[Routing] fixed possible parameters conflict in apache url matcher
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (as long as rewrite rules are generated after upgrading)
Symfony2 tests pass: yes
- This fixes a conflict in route parameters:
The rewrite rules currently pass route informations through environment variables:
`_ROUTING_DEFAULT_x`: passes the default value of parameter x
`_ROUTING__allow_x`: passes the information that method x was allowed for this route
`_ROUTING_x`: passes the value of parameter x
The problem is that naming a route parameter `DEFAULT_*` or `_allow_*` would not behave as expected.
I fixed this by namespacing all environment variables; e.g. parameters are in `_ROUTING_param_*`, defaults in `_ROUTING_default_*`, etc.
- The PR fixes a second issue: sometimes the variables are prefixed with multiple REDIRECT_. This PR handles this case by ignoring them all.
- This also improves performance a little:
Matching a route with two parameters and two default parameters 100K times: (`$_SERVER` was copied from a real request, so with many non `_ROUTING_` variables)
master: 6.6s
this branch: 4.7s
---------------------------------------------------------------------------
by fabpot at 2012-10-27T13:37:24Z
Any news on this PR? Is it mergeable?
---------------------------------------------------------------------------
by arnaud-lb at 2012-10-27T14:50:08Z
There is an issue with default parameter values, I can't find how to fix that in a simple way. Before this PR, default values are never used (if a parameter is an optional not present in the url, the parameter's value is the empty string); after this PR, when a parameter is present and empty (e.g. a requirement like `.*`), its value is set to its default value.
---------------------------------------------------------------------------
by Tobion at 2012-10-29T01:36:08Z
The problem is, it's not consistent with the default php matcher. So one cannot safely exchange it with the apache matcher because it behaves differently under some (special) circumstances.
---------------------------------------------------------------------------
by fabpot at 2012-11-05T08:05:54Z
We need to move forward as I want to merge the hostname support in the routing ASAP to have plenty of time for feedback before the 2.2 release.
Does it sound reasonable to merge this PR as is an open a ticket about the remaining issue (which should not occur that often anyways)?
---------------------------------------------------------------------------
by arnaud-lb at 2012-11-05T09:22:02Z
@fabpot it sounds reasonable to me. Also, I've the hostname support branch is currently rebased so that it can be merged without this one.
---------------------------------------------------------------------------
by Tobion at 2012-11-11T21:50:20Z
Btw, does the ApacheMatcherDumper handle the _scheme requirement? It doesn't look like it. This would be another bug.
Anyway, we can probably merge this PR and open new issues for the remaining bugs.
This PR was merged into the master branch.
Commits
-------
d0e5ef1 [FrameworkBundle] move change note to changelog of FrameworkBundle instead of master UPGRADE-2.2
e77ecc9 [FrameworkBundle] switch to parameter for base url to make it configurable for tests and cli as done for host and scheme in d30943c2e8
Discussion
----------
[FrameworkBundle] switch to parameter for base url
This will make it configurable the same way we have it for host and scheme done for 2.1 in d30943c2e8
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: (no new test breakage)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: (none yet. if this is merged, i will happily PR against doc if somebody can point me to the right place)
---------------------------------------------------------------------------
by dbu at 2012-11-11T18:44:33Z
doc will go here once its merged: http://symfony.com/doc/current/cookbook/console/generating_urls.html
---------------------------------------------------------------------------
by dbu at 2012-11-11T20:12:41Z
ok, moved to the FrameworkBundle changelog
---------------------------------------------------------------------------
by Tobion at 2012-11-11T20:34:28Z
unfortunately is naming is so wrong in many places of the Request class and the routing.
`base_url` is just an example. It's not an "URL" at all. It's a path only.
---------------------------------------------------------------------------
by dbu at 2012-11-12T07:47:55Z
@Tobion i saw that, but i chose to be consistent with the method name of RequestContext even if it is a wrong name.
This PR was merged into the master branch.
Commits
-------
e32ca2b [HttpKernel] Fix Symfony2 full framework tests
Discussion
----------
[HttpKernel] Fix Symfony2 full framework tests
Fix the path when the full framework is used
---------------------------------------------------------------------------
by fabpot at 2012-11-12T09:08:06Z
When is it broken?
---------------------------------------------------------------------------
by vicb at 2012-11-12T09:18:01Z
now, https://travis-ci.org/symfony/symfony/jobs/3159326
This PR was merged into the master branch.
Commits
-------
2972f31 [HttpKernel] Changlog update - document a BC break
Discussion
----------
[HttpKernel] Changlog update - document a BC break
This PR was squashed before being merged into the master branch (closes#5970).
Commits
-------
d0433b6 [Stopwatch] Get the "real size" used & minor tweaks
Discussion
----------
[Stopwatch] Get the "real size" used & minor tweaks
---------------------------------------------------------------------------
by vicb at 2012-11-11T09:45:50Z
@fabpot @maoueh thanks for your feedback, integrated.
This PR was merged into the master branch.
Commits
-------
02315a0 Removed duplicate line
Discussion
----------
Removed duplicate line
This just removes a duplicate line in ```composer.json``` file.
Thanks.
This PR was merged into the master branch.
Commits
-------
e13b050 Add missing symfony/stopwatch in the replace section of composer.json
Discussion
----------
Add missing symfony/stopwatch in the replace section of composer.json
And order the sections alphabeticaly for easier navigation
This PR was merged into the master branch.
Commits
-------
fc300ec [FrameworkBundle] Router class tweak
Discussion
----------
[FrameworkBundle] Router class tweak
This PR was merged into the master branch.
Commits
-------
4c0c588 [MemoryDataCollector] Stop being optimistic about memory usage
Discussion
----------
[MemoryDataCollector] Stop being optimistic about memory usage
Take into account the memory used in the kernel.terminate listeners
This PR was merged into the master branch.
Commits
-------
b27b749 made usage of Composer autoloader for subtree-split unit tests
Discussion
----------
made usage of Composer autoloader for subtree-split unit tests
This PR also normalizes the way components are tested.
---------------------------------------------------------------------------
by stof at 2012-11-09T23:14:22Z
👍
This PR was merged into the master branch.
Commits
-------
a288521 displayed the full event listener names to avoid name collisions (closes#5019)
Discussion
----------
displayed the full event listener names to avoid name collisions (closes#5019)
---------------------------------------------------------------------------
by stof at 2012-11-09T23:14:41Z
👍
This PR was merged into the master branch.
Commits
-------
6b54a51 moved the Stopwatch classes to their own component
Discussion
----------
moved the Stopwatch classes to their own component
see https://github.com/symfony/symfony/issues/5911
---------------------------------------------------------------------------
by stof at 2012-11-09T08:06:06Z
you should also add the ``.gitattributes`` in the new component
---------------------------------------------------------------------------
by fabpot at 2012-11-09T09:09:35Z
@stof: I've added the .gitattributes file, but also the phpunit and LICENSE ones.
---------------------------------------------------------------------------
by dlsniper at 2012-11-09T13:51:23Z
Also you should add this to the changelog/update files. I'll do it myself but only in about 8 hours or so as I'm not at home.
This PR was merged into the master branch.
Commits
-------
380cf4f [HttpKernel] added memory information in the Stopwatch
Discussion
----------
[HttpKernel] added memory information in the Stopwatch
This PR was merged into the master branch.
Commits
-------
d231b8f [ClassLoader] Make ApcClassLoader properly decorate ClassLoader.
Discussion
----------
[ClassLoader] Make ApcClassLoader properly decorate ClassLoader.
ApcClassLoader only implements the findFile() and loadClass() methods, but none of the other methods. That's an implementation detail, which isn't particularly trivial.
I can't see a reason for why ApcClassLoader cannot simply decorate ClassLoader in a proper way, passing forward all unknown method calls to ClassLoader.
---------------------------------------------------------------------------
by sun at 2012-11-09T03:00:51Z
mmm, the test failures seem to be irrelevant. It passed on the PHP 5.4 environment. The others failed on early pre-test infrastructure operations.
