This PR was merged into the master branch.
Commits
-------
7fe44da Whitespace corrections
6d30f20 Switched to using a method to get original class name that did not require string parsing
3c8d607 Changed test to use a longer form, complete check of the contents of the trace
de77c88 Whitespace correction
03a7bb9 Added a unit test to verify incomplete classes do not cause flatten exception to throw
e562418 Added a bit to convert incomplete objects in the error message
Discussion
----------
[HttpKernel] Added a bit to convert incomplete objects in the error message
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes*
\* no - errors in MongoDbSessionHandlerTest attempting to access private property, however running just src/Symfony/Component/HttpKernel does pass.
Objects of class __PHP_Incomplete_Class are only sometimes an object.
```
$object = unserialize('O:14:"BogusTestClass":0:{}');
$object instanceof __PHP_Incomplete_Class; // true
is_object($object); // false
gettype($object); // "object"
```
Since it is "not an object", the flatter attempts to turn it into a string, it triggers:
```
__PHP_Incomplete_Class could not be converted to string.
```
Which then hides the root error message.
---------------------------------------------------------------------------
by pborreli at 2012-09-18T16:16:33Z
have you seen that http://stackoverflow.com/questions/965611/forcing-access-to-php-incomplete-class-object-properties looks like you can still access the object even if it's a __PHP_Incomplete_Class with foreach
---------------------------------------------------------------------------
by rrehbeindoi at 2012-09-18T16:38:38Z
Thank you for the tip re: foreach.
* 2.1:
Added Base64 encoding, decoding to MongoDBProfilerStorage
Fix duplicated code and a field name
refactor src/Symfony/Component/Translation/Loader/MoFileLoader.php
fixed typo
Update src/Symfony/Component/Validator/Resources/translations/validators.pl.xlf
fixed issue #5596 (Broken DOM with the profiler's toolbar set in position top)
[Form] Fixed the testsuite for PHPUnit 3.6 as travis still uses it
added dirs generated by build-data.php in locale component to .gitignore
[Process] Fixed bug introduced by 7bafc69f38.
[Process][Tests] Prove process fail (Add more test case)
[Process][Tests] Prove process fail
[HttpFoundation] Fixed the tests
[DomCrawler] Added test for supported encodings by mbstring
[Config] Fixed preserving keys in associative arrays
[Console] Fixed return value for Command::run
[Locale] Fixed tests
[Console] Fix some input tests
[Filesystem] Fixed tests on Windows
[Config] Fixed tests on Windows
This PR was merged into the 2.1 branch.
Commits
-------
d7623ae [DomCrawler] Added test for supported encodings by mbstring
Discussion
----------
[2.1][DomCrawler] Added test for supported encodings by mbstring
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
---------------------------------------------------------------------------
by fabpot at 2012-09-25T09:35:18Z
As this is a bug fix, it should be done on the 2.0 branch. Thanks.
---------------------------------------------------------------------------
by stof at 2012-09-25T09:41:59Z
@fabpot 2.0 does not contain the code trying to convert the encoding.
Commits
-------
27b2df9 [Process] Fixed bug introduced by 7bafc69f38.
7a955c0 [Process][Tests] Prove process fail (Add more test case)
598dcf3 [Process][Tests] Prove process fail
Discussion
----------
[Process][Tests] Prove process fail with chained commands
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: no
Fixes the following tickets: -
Todo: Fix that
License of the code: MIT
This PR is against 2.1 branch. Previous PR was #5575
This PR try to hiligh a regression in Process component.
``` php
$process = new Process("echo -n 1 && echo -n 1");
// or $process = new Process("echo -n 1 ; echo -n 1");
$process->run();
var_dump('11' == $process->getOutput()); // false,
var_dump($process->getOutput()); // '1',
```
This test failed because of PR #5543 ; see 7bafc69f38 (L0R233)
---------------------------------------------------------------------------
by romainneutron at 2012-09-25T13:05:45Z
You've to revert the change that causes the fail (ie: remove https://github.com/symfony/symfony/blob/2.1/src/Symfony/Component/Process/Process.php#L233)
---------------------------------------------------------------------------
by romainneutron at 2012-09-25T13:06:56Z
BTW, removing this line re-open #5030
---------------------------------------------------------------------------
by stof at 2012-09-25T13:11:15Z
@lyrixx please add a commit reverting the addition of ``exec`` in the case of sigchild not being used (only this addition, not the full commit you linked) as it should fix your test.
---------------------------------------------------------------------------
by stof at 2012-09-25T13:12:21Z
@fabpot btw, this regression is quite important. As I said in the previous PR, it impacts composer in a bunch of places.
---------------------------------------------------------------------------
by romainneutron at 2012-09-25T13:30:07Z
You reverted too much things, you just had to remove line 233
---------------------------------------------------------------------------
by stof at 2012-09-25T13:42:49Z
@lyrixx I explicitly asked you to revert only the ``exec`` addition for the case without sigchild.
---------------------------------------------------------------------------
by lyrixx at 2012-09-25T13:55:57Z
@stof Sorry, i fixed that.
---------------------------------------------------------------------------
by romainneutron at 2012-09-25T13:56:26Z
@lyrixx just remove the two last commit, edit Process.php and remove line 233
---------------------------------------------------------------------------
by lyrixx at 2012-09-25T13:59:59Z
@romainneutron I think it's ok now.
---------------------------------------------------------------------------
by romainneutron at 2012-09-25T14:11:28Z
yep it's good :)
Commits
-------
2dcb2d7 [Filesystem] Fixed tests on Windows
Discussion
----------
[2.1][Filesystem] Fixed tests on Windows
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Commits
-------
fc1e844 [Locale] Fixed tests
Discussion
----------
[2.1][Locale] Fixed tests
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Commits
-------
65281fb [Config] Fixed tests on Windows
Discussion
----------
[2.1][Config] Fixed tests on Windows
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Commits
-------
b961ee3 [HttpFoundation] Fixed the tests
Discussion
----------
[HttpFoundation] Fixed the tests
b8a2f8c646 reverted the use of the username
and password in the getSchemeAndHost method but forgot to revert the
corresponding tests.
Commits
-------
8ab3054 added dirs generated by build-data.php in locale component to .gitignore
Discussion
----------
added dirs generated by build-data.php in locale component to .gitignore
This is to complete the PR #5411.
Paging @eriksencosta.
---------------------------------------------------------------------------
by eriksencosta at 2012-09-25T14:54:06Z
For me it's ok!
Batman?
---------------------------------------------------------------------------
by shieldo at 2012-09-25T14:55:38Z
Kapow! Thanks for checking it over!
---------------------------------------------------------------------------
by shieldo at 2012-09-25T15:41:05Z
As @stof pointed out, git does read .gitignore files in sub-paths. So I've modified the commit so the change is in the Locale component only.
Commits
-------
530bd22 fixed issue #5596 (Broken DOM with the profiler's toolbar set in position top)
Discussion
----------
fixed issue #5596 (Broken DOM with the profiler's toolbar set in position top)
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #5596
Todo: -
License of the code: MIT
Documentation PR: -
Whatever the toolbar position, the html code associated to it may be placed at the end of the page (and this will be better from a webperf point of view).
---------------------------------------------------------------------------
by stof at 2012-09-25T17:47:53Z
The spacer div will not be in the right place when using ``position: top`` as it will be at the end of the body.
---------------------------------------------------------------------------
by pborreli at 2012-09-25T17:53:03Z
@stof what is the spacer div ? i guess all the profiler is in absolute so i guess it should work (and I'm sure @xavierlacot already tested it :))
---------------------------------------------------------------------------
by pborreli at 2012-09-25T17:55:55Z
@xavierlacot so maybe we should refactor
```
$pos = $posrFunction($content, '</body>');
$content = $substrFunction($content, 0, $pos).$toolbar.$substrFunction($content, $pos);
```
with something like
```
$content = str_replace('</body>', $toolbar.'</body>', $content);
```
What do you think ?
---------------------------------------------------------------------------
by stof at 2012-09-25T17:58:35Z
@pborreli The toolbar is in position fixed. But to avoid hiding some of the content of your page, another div is added on with a margin, to force keeping some space after the content for the toolbar. With this change, the toolbar HTML is always at the end, so the 40px space is always added at the bottom of the page even if the toolbar is added at the top.
---------------------------------------------------------------------------
by pborreli at 2012-09-25T18:03:08Z
@stof maybe we should just fix the body/html margin-top in that case no ? or find a better solution, anyway I think the actual way to do it is bad, `<body>` and `</body>` are not even mandatory in html5, IMHO i would just put it at the end of file without any check, then fix it with some css and/or js
---------------------------------------------------------------------------
by stof at 2012-09-25T18:06:58Z
@pborreli Putting it at the end after the closing ``<html>`` tag would make the page invalid for people defining the markup fully. It is a bad idea.
And anyway, detecting the body tag is still important, to avoid injecting the toolbar in partial page content (be it ESI requests or parts loaded through AJAX).
Oh, and ``$content = str_replace('</body>', $toolbar.'</body>', $content);`` would not fix#5596 but make it worse: it would also inject the toolbar in the head even when being placed at the bottom (keeping it at the bottom too).
---------------------------------------------------------------------------
by pborreli at 2012-09-25T18:18:46Z
@stof for detecting ajax you already have `if ($request->isXmlHttpRequest()) {` called few lines before,
the proposal for `$content = str_replace('</body>', $toolbar.'</body>', $content);` was only a refactoring for the above PR
---------------------------------------------------------------------------
by stof at 2012-09-25T18:26:34Z
@pborreli ESI requests are not AJAX requests. So simply appending at the end would still break them.
And your code is *not* a refactoring. Your ``str_replace`` will replace **all** occurences of ``</body>``, not just the last one. See the related issue to understand why it makes a difference
---------------------------------------------------------------------------
by pborreli at 2012-09-25T18:38:11Z
ok I'm all wrong.
---------------------------------------------------------------------------
by xavierlacot at 2012-09-25T18:51:42Z
@stof, please review the last commit, which injects the wdt container at the top of the page in javascript, using the browser's DOM capacities. This fixes the spacer problem that you noticed.
---------------------------------------------------------------------------
by stof at 2012-09-25T18:55:51Z
Well, you are now breaking things when the spacer should be at the bottom as you are always putting the spacer at the top.
---------------------------------------------------------------------------
by stloyd at 2012-09-25T19:06:14Z
@xavierlacot Pass `position` variable to template and change:
```diff
- document.body.insertBefore(sfwdt, document.body.firstChild);
+ {% if position == 'bottom' -%}
+ document.body.appendChild(sfwdt);
+ {%- else -%}
+ document.body.insertBefore(sfwdt, document.body.firstChild);
+ {%- endif %}
---------------------------------------------------------------------------
by stof at 2012-09-25T20:18:31Z
@xavierlacot could you squash your commits ?
---------------------------------------------------------------------------
by xavierlacot at 2012-09-25T21:32:47Z
@stof done. Thanks for the review :-)
Commits
-------
ef288a2 [Form] Fixed the testsuite for PHPUnit 3.6 as travis still uses it
Discussion
----------
[Form] Fixed the testsuite for PHPUnit 3.6
Travis is still using it so this avoids making all build fail just because of it.
Commits
-------
c869a65 [Console] Fixed return value for Command::run
Discussion
----------
[2.1][Console] Fixed return value for Command::run
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
API says that Command::run returns integer. This is also necessary if I want to run nested commands.
* 2.1:
fixed stringification of array objects in RequestDataCollector (closes#5295)
[HttpFoundation] removed the username and password from generated URL as generated by the Request class (closes#5555)
[Console] fixed default argument display (closes#5563)
Fixing config normalisation example in docblock
* 2.0:
fixed stringification of array objects in RequestDataCollector (closes#5295)
Fixing config normalisation example in docblock
Conflicts:
src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php
src/Symfony/Component/HttpKernel/DataCollector/RequestDataCollector.php
Quoted from the ticket it solves for future reference:
"I've been having issues with using htdigest auth (requirement for me to
work with) after upgrading to 2.1. Each time a resource is loaded, a
prompt is given for the HTTP Auth username and password, and Chrome does
not automatically respond to these 401 responses with the credentials it
already has. I've traced the issue to being caused by the HttpFoundation
Component, specifically Request.php.
The request class adds the PHP_AUTH_USER/PHP_AUTH_PW parameters to the
request URI (changes http://www.mysite.com requests to
http://user:pw@www.mysite.com) in getSchemeAndHttpHost(). This behaviour
is not specified in the HTTP RFC, and is incompatible with Chrome as of
Chrome 19, IE (as of IE 9) and has special behaviour in Firefox (prompts
the user to confirm they know they're logging into the site, which is an
ambiguous behaviour at best, but at least it's something if they're
going to support it for now).
This functionality was added about to HttpFoundation about a year ago,
but it really should be removed and standard protocol practices should
be followed. This practice makes it possible for cross-site tracking and
other malicious behaviours to be performed by hiding information in the
authorization headers, which explains why most browsers no longer
support or take exception with it.
The offending line is specifically this. Replacing it with return
$this->getScheme().'://'.$this->getHttpHost(); seems to solve the
problem."
Commits
-------
6bafe5a moved some code to the component
Discussion
----------
moved some code to the component
This simply moves some code from a command to a dedicated class to make it more reusable.
I wasn't able to run tests because composer failed to install dependencies. Let's see if travis can do better.
* 2.1:
bumped Symfony version to 2.1.3-DEV
updated VERSION for 2.1.2
updated CHANGELOG for 2.1.2
Fixed FlashBagInterface phpdoc, clarified UPGRADE docs
composer is available in travis workers
Conflicts:
src/Symfony/Component/HttpKernel/Kernel.php
Commits
-------
bb0e4c3 Fixed FlashBagInterface phpdoc, clarified UPGRADE docs
Discussion
----------
Fixed FlashBagInterface phpdoc, clarified upgrade doc
The fact that multiple flash messages are now stored/retrieved per type was an additional BC break that I missed when I first went through the 2.1 update doc, and it didn't help that the phpdoc was outdated.
These changes just clarify things a little.
---------------------------------------------------------------------------
by drak at 2012-09-20T07:45:52Z
+1
Commits
-------
71db836 Better config validation handling for numerical values: * New node type Integer and Float * New expressions: min() and max()
Discussion
----------
[2.2] [Config] Better handling for numerical values:
* New node type Integer and Float
* New expressions: ifLessThan(), ifGreaterThan(), ifInRange(), ifNotInRange()
---------------------------------------------------------------------------
by fabpot at 2012-07-03T08:50:22Z
As I said on PR #4713, adding more method clutters the API without any big benefits. I'm -1 on the PR.
---------------------------------------------------------------------------
by jeanmonod at 2012-07-03T08:54:36Z
I have been discuss it with @schmittjoh at the sflive, he was thinking it could be a good addition.
IMHO I think that if we want to encourage the usage of bundle configuration validation, we should make it as easy as possible to use...
---------------------------------------------------------------------------
by jeanmonod at 2012-07-03T08:59:42Z
A real life example:
->scalarNode('max_nb_items')
->validate()
->ifTrue(function($v){
return !is_int($v) || (is_int($v) && $v<1);
})
->thenInvalid('Must be a positive integer')
->end()
->end()
could be replaced by
->integerNode('max_nb_items')
->validate()
->ifLessThan(1);
->thenInvalid('Must be a positive integer')
->end()
->end()
---------------------------------------------------------------------------
by gnutix at 2012-07-03T09:03:06Z
I agree with @jeanmonod on this matter, the bundle configuration validation is already kind of a hassle to understand (and read), so it would be a good addition IMHO.
---------------------------------------------------------------------------
by fabpot at 2012-07-03T10:54:32Z
@schmittjoh What's your point of view?
---------------------------------------------------------------------------
by schmittjoh at 2012-07-03T14:10:37Z
The integer and float nodes are valuable additions imo which I wanted to add myself several times but simply did not have the time for.
As for the changes to the expression builder, I was not really passionate about them in Paris, but I did not mind either way. However, looking at this PR, I think they would be better implemented as methods on the definition builders, and validated directly by the nodes:
```php
$builder->integerNode('foo')->range(1, 4)->end();
$builder->integerNode('foo')->mustBeGreaterThan(5)->end();
```
This will also allow for these constraints to be introspected and added to generated documentation.
---------------------------------------------------------------------------
by fabpot at 2012-07-03T17:57:25Z
@jeanmonod Can you take into account the comments by @schmittjoh?
---------------------------------------------------------------------------
by jeanmonod at 2012-07-03T19:40:24Z
@fabpot Yes, I will try to move those 4 checks.
@schmittjoh If I put those tests into the ScalarNodeDefinition did you think it's ok? And did I have to make anything special for the documentation introspection?
---------------------------------------------------------------------------
by schmittjoh at 2012-07-03T19:56:00Z
You can take a look at the EnumNodeDefinition, and the EnumNode. They are
pretty simple, and should give you a good idea of how to implement it.
On Tue, Jul 3, 2012 at 9:46 PM, Jeanmonod David <
reply@reply.github.com
> wrote:
> @fabpot Yes, I will try to move those 4 checks.
>
> @schmittjoh If I put those tests into the ScalarNodeDefinition did you
> think it's ok? And did I have to make anything special for the
> documentation introspection?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/4714#issuecomment-6744309
>
---------------------------------------------------------------------------
by jeanmonod at 2012-07-03T21:37:18Z
OK, I just refactor as requested. At the end, I didn't add the range() check. It can be easily done by chaining min and max, like this:
$builder->integerNode('foo')->min(1)->max(4)->end();
@schmittjoh can you have a look?
---------------------------------------------------------------------------
by schmittjoh at 2012-07-03T21:48:17Z
Have you tested the builder API? Did you maybe forget to commit something?
---------------------------------------------------------------------------
by jeanmonod at 2012-07-03T21:52:45Z
Yes you are right, I forget the definition
---------------------------------------------------------------------------
by jeanmonod at 2012-07-03T22:15:45Z
OK, I realize now that I misunderstood the concept. I was thinking that a node was able to do self validation. But no, I will have to move my code to the node definition. So let's wait for a new commit...
---------------------------------------------------------------------------
by jeanmonod at 2012-07-06T06:13:55Z
@schmittjoh I just push the move to definition and the new abstract class Numeric. Can you review it?
---------------------------------------------------------------------------
by jeanmonod at 2012-07-10T05:12:59Z
@schmittjoh, @fabpot
I fix all the mention points, can you have a look at the final result?
---------------------------------------------------------------------------
by schmittjoh at 2012-07-10T06:38:20Z
There are still some excessive blank lines if you want to be perfect, but overall looks good now.
---------------------------------------------------------------------------
by jeanmonod at 2012-07-10T07:05:54Z
@schmittjoh I think the comments of @Baachi are not well placed in the diff. I execute php-cs-fix on all code, so level of perfectness is already good ;)
@fabpot Do you want some more complements before merging?
---------------------------------------------------------------------------
by fabpot at 2012-07-10T07:07:21Z
@jeanmonod I'm going to review the code once more and it will be merged for 2.2. Thanks for your work.
---------------------------------------------------------------------------
by fabpot at 2012-09-18T13:58:48Z
@jeanmonod Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by jeanmonod at 2012-09-18T14:36:59Z
@fabpot Squash done
---------------------------------------------------------------------------
by fabpot at 2012-09-19T04:07:13Z
@jeanmonod One last thing: can you submit a PR on symfony/symfony-docs that update the documentation with the new possibilities?
---------------------------------------------------------------------------
by jeanmonod at 2012-09-20T05:32:01Z
@fabpot OK, Documentation PR done here: https://github.com/symfony/symfony-docs/pull/1732
* 2.1:
[Form] removed comment now that PHPUnit 3.7 is out
Add a Sigchild compatibility mode (set to false by default)
fix Fatal error: Cannot access private property
Conflicts:
src/Symfony/Component/Process/Process.php
Commits
-------
bb2566c [Console] Console colorization is also provided by ConEmu on Windows
Discussion
----------
[Console] Console colorization is also provided by ConEmu on Windows
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
See http://code.google.com/p/conemu-maximus5/wiki/AnsiEscapeCodes#Переменная_окружения
---------------------------------------------------------------------------
by fabpot at 2012-09-16T07:43:55Z
Can you add a note about it in the changelog of the component? thanks
Commits
-------
e271b17 Remove the string optimization since it causes no real performance gain but increases generation time of the dumped PHP Container
Discussion
----------
PhpDumper and large strings
When the PhpDumper is dealing with longer strings, the regular expression performed to optimize this can be quite a performance hog.
In our case sometimes the dumper takes more then 30 seconds if leaving this enabled. Disabling it will bring it back to sub-second speed.
This patch makes the optimization optional by passing in an additional container option.
---------------------------------------------------------------------------
by fabpot at 2012-08-25T16:57:29Z
I don't like adding yet another option for something that should "just works". It would be better to find a better way to optimise the strings for all cases.
---------------------------------------------------------------------------
by mazen at 2012-08-25T17:22:07Z
I never really tested how much of a runtime difference it incurs when either using the "optimized" version or the non optimized version, so:
Having an example at hand which generates stable results yields (in a non-debug environment with a booted container using either of the optimization methods):
Without optimized strings:
```
Time taken for tests: 14.865 seconds
Complete requests: 1000
Failed requests: 0
Write errors: 0
Total transferred: 217000 bytes
HTML transferred: 8000 bytes
Requests per second: 67.27 [#/sec] (mean)
Time per request: 14.865 [ms] (mean)
Time per request: 14.865 [ms] (mean, across all concurrent requests)
Transfer rate: 14.26 [Kbytes/sec] received
Connection Times (ms)
min mean[+/-sd] median max
Connect: 0 0 0.0 0 0
Processing: 14 15 19.7 14 632
Waiting: 14 15 19.7 14 632
Total: 14 15 19.7 14 632
Percentage of the requests served within a certain time (ms)
50% 14
66% 14
75% 14
80% 14
90% 14
95% 14
98% 15
99% 23
100% 632 (longest request)
```
With Optimized Strings enabled
```
Time taken for tests: 14.077 seconds
Complete requests: 1000
Failed requests: 0
Write errors: 0
Total transferred: 217000 bytes
HTML transferred: 8000 bytes
Requests per second: 71.04 [#/sec] (mean)
Time per request: 14.077 [ms] (mean)
Time per request: 14.077 [ms] (mean, across all concurrent requests)
Transfer rate: 15.05 [Kbytes/sec] received
Connection Times (ms)
min mean[+/-sd] median max
Connect: 0 0 0.0 0 0
Processing: 14 14 1.3 14 48
Waiting: 14 14 1.3 14 48
Total: 14 14 1.3 14 48
Percentage of the requests served within a certain time (ms)
50% 14
66% 14
75% 14
80% 14
90% 14
95% 14
98% 14
99% 15
100% 48 (longest request)
```
So the response times differ by around 0.8ms
Building the non-optimized container takes around 800ms
Building the optimized container takes 43 seconds
From my Point of View it would just be viable to remove the optimization (since it already incurred some issues fixed in 808088a3ca).
I do not see a way how to improve the regexps (but by all means I'm no regular expression guru)
---------------------------------------------------------------------------
by fabpot at 2012-08-30T07:12:55Z
I'm also for removing these optimizations. What others think?
---------------------------------------------------------------------------
by Baachi at 2012-08-30T07:54:53Z
I'm +1 for removing this feature.
The performance boost is to small.
Commits
-------
9135431 [HttpKernel] Added support for WinCache in ConfigDataCollector
Discussion
----------
[HttpKernel] Added support for WinCache in ConfigDataCollector
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Commits
-------
f2e4802 [Yaml] Normalize exceptions
b0f5f2e [Serializer] Normalize exceptions
bcd8db2 [DependencyInjection] Normalize exceptions
Discussion
----------
Normalize exceptions
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
This PR adds consistence to components which already have their own exception interface.
DependencyInjection, Serializer and Yaml now only throw their own scoped exceptions.
For other components, it's much more work and could introduce some bugs. It would be better to do it in Symfony 2.2.
Commits
-------
c5e7793 [Process] Normalize exceptions
Discussion
----------
[Process] Normalize exceptions
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
There were some exceptions in the Process scope but they were not implemented everywhere in the component.
This PR ensure that all exceptions thrown inside Process implements `Process\Exception\ExceptionInterface`.
---------------------------------------------------------------------------
by romainneutron at 2012-08-30T20:05:41Z
Tests passes, it's a travis issue, see http://travis-ci.org/#!/symfony/symfony/builds/2287439
PHP Fatal error: Cannot access private property Symfony\Component\HttpFoundation\Tests\Session\Storage\Handler\MongoDbSessionHandlerTest::$options
in src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php on line 85
Commits
-------
22e9036 updated CHANGELOG
bafe890 [FrameworkBundle] changed Client::enableProfiler() behavior to fail silently when the profiler is not available (it makes it easier to write functional tests)
f41872b [FrameworkBundle] added a way to enable the profiler for the very next request in functional tests (closes#4307)
67b91e5 [HttpKernel] added a way to enable a disable Profiler
Discussion
----------
[2.2] added a way to enable the profiler for the very next request in a functional test
Bug fix: yes/no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4307
Todo: -
License of the code: MIT
Documentation PR: should be done before merging
After merging this PR, we need to disable the profiler in the test environment in Symfony SE.
Commits
-------
6ff9b04 Add Luhn validator
Discussion
----------
[2.2] [Validator] Add Luhn validator
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: Adding documentation if this PR is blessed.
License of the code: MIT
I submitted these validators to JMSPaymentCoreBundle, because they're payment related, but @schmittjoh feels that they're a better choice for adding to Symfony2.
See schmittjoh/JMSPaymentCoreBundle#52 for the original submission.
---------------------------------------------------------------------------
by fabpot at 2012-07-04T05:19:42Z
I'm +1. @bschussek?
---------------------------------------------------------------------------
by shieldo at 2012-07-04T15:46:05Z
+1 in principle (certainly the Luhn check). I did wonder whether Visa Electron should also be in there (although, that uses a subset of the Visa range, and the chance that you would be checking for a Visa Electron but *not* Visa at the same time is vanishingly small, so maybe this is unnecessary).
---------------------------------------------------------------------------
by merk at 2012-07-04T21:25:30Z
@shieldo I did have a concern about the Electron as well, but in the case of an online system doing payment processing, I'm not sure anyone would ever need to check if it was an Electron card or not.
---------------------------------------------------------------------------
by merk at 2012-07-04T21:28:25Z
We could expand the CardScheme stuff further with this list: http://en.wikipedia.org/wiki/List_of_Issuer_Identification_Numbers
Is there any point expanding the validator beyond financial services and into the other sevices listed?
---------------------------------------------------------------------------
by shieldo at 2012-07-04T21:37:41Z
@merk Yes, in actuality there are always going to be cases you can't trap with a regex - I'd say validation like this is working if it catches a majority of cases of invalid numbers, because in reality ones that get through will just fail downstream anyway. The purpose of the validator isn't to identify individual schemes for numbers, it's to do a sanity check across collections of schemes.
I don't really see any point expanding beyond financial services for a core validator (imho) - I'm not sure how stable some of this information is.
---------------------------------------------------------------------------
by Gator92 at 2012-07-11T00:45:50Z
+1 on the Luhn check (without the authorship, just give credit to Greg Knapp), the CardScheme, however, is not really required by most gateways these days, it's a better candidate for a custom constraint.
---------------------------------------------------------------------------
by merk at 2012-07-11T00:47:20Z
I dont object, but Greg's algorithm has a flaw for odd length creditcard numbers. The unit testing written by Infinite caught this.
---------------------------------------------------------------------------
by Gator92 at 2012-07-11T01:35:46Z
You're right, the Knapp algo is flawed and does not appear to work on odd-numbered length cards.
---------------------------------------------------------------------------
by fabpot at 2012-07-11T05:49:22Z
@merk: Can you open two new pull requests? One for each validator?
---------------------------------------------------------------------------
by merk at 2012-07-26T23:42:24Z
PR updated to remove CardScheme into its own PR.
Documentation PR added to symfony-docs
* 2.1:
Create CONTRIBUTING.md file for auto-linking in PR's
Added Bulgarian translation
[Profiler]Use the abstract method to get client IP
Typo fix
Fixing incorrect word in twig:lint command description
Rename $key parameter to $name for consistency
=Minor chnage: replaced function by method
Fixed the phpdoc in the DependencyInjection component
Commits
-------
02516de [Routing] fix variable with a requirement of '0'
1f5b793 [Routing] fix setting empty requirement in Route
Discussion
----------
[Routing] fix setting empty requirement
First commit: A requirement of "^$" was overlooked and wasn't recognized as empty after stripping it in Route.
Second commit: Fixes a requirement of '0' that was ignored by the Compiler.
