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.
Commits
-------
cc46780 [Console] Fix some input tests
Discussion
----------
[Console] Fix some input tests
New PR for console input tests, was previously on the master branch : https://github.com/symfony/symfony/pull/5567
* 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
-------
589a8b3 composer is available in travis workers
Discussion
----------
composer is available in travis workers
---------------------------------------------------------------------------
by bamarni at 2012-09-19T09:07:30Z
reverted
---------------------------------------------------------------------------
by stof at 2012-09-19T09:59:21Z
btw, in the 2.1 branch, the ``COMPOSER_ROOT_VERSION`` varaible should be set to ``2.1.x-dev``
---------------------------------------------------------------------------
by bamarni at 2012-09-19T14:44:48Z
@stof: ok, btw I don't understand the issue, deps are solvable without this, is it a workaround to make this package known as 2.1 even though it's not guessable with the branch name (eg. when using a feature branch for a PR)? I thought this was handled by the branch-alias.
---------------------------------------------------------------------------
by stof at 2012-09-19T15:54:05Z
@bamarni It is solvable in some cases but not in all of them. Sometimes, when being in a detached head (which is the case when Travis builds PRs), the guessing of the composer root version does not work, and so symfony dev deps depending on some symfony components (Doctrine) will fail because the guessed version would not match ``2.*`` (which is the Doctrine requirement for the components).
branch aliases have nothing to do with the guessing of the version for the root package.
---------------------------------------------------------------------------
by stof at 2012-09-19T15:55:25Z
btw, the only cases where COMPOSER_ROOT_VERSION could be needed is when some of your dependencies have a requirement on your root package. Otherwise, the root version is never needed by composer to resolve deps
---------------------------------------------------------------------------
by bamarni at 2012-09-19T16:52:52Z
@stof: in detached head indeed the version can't be guessed with git, so this change avoid potential issues with reciprocal dependencies (I guess it can also be solved by specifying a "version" in composer.json).
But I still don't get why you say branch aliases have nothing to do with the root package version, isn't the purpose of a "2.1-dev" branch-alias to be matched when specifying this package as a dependency with 2.1.*, no matter the branch name or if git is in detached head?
---------------------------------------------------------------------------
by stof at 2012-09-19T18:59:13Z
@bamarni a **branch** alias is about the aliasing the version of a branch. Look at how it is written. It tells composer that for Symfony, the ``dev-master`` version (so the master branch) should receive an alias ``2.2.x-dev``.
This alias will never help you if your root version cannot be guessed as being ``dev-master``.
---------------------------------------------------------------------------
by bamarni at 2012-09-19T20:43:19Z
ahh... in my mind branch alias was only an alias for the current branch, I forgot that it was a 'branch => alias' map, indeed in that case it can't help at all.
thx for the explanations ;)
Commits
-------
6cbeff0 use ->find instead of ->get in the help command to allow command aliases to be used (e.g. "./app/console help do:sc:ge")
Discussion
----------
use ->find instead of ->get in the help command to allow command aliases...
... to be used (e.g. "./app/console help do:sc:ge")
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
-------
7bafc69 Add a Sigchild compatibility mode (set to false by default)
Discussion
----------
[2.1][Process] Fix stop in non-sigchild environments
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Fix#5030 in half way.
- `proc_terminate` now sends the `SIGTERM` to the real process, not the sh (add the exec prefix to remove the wrapper as suggested by @schmittjoh). So now, process will stop, except if you're working with a PHP compiled with `--enable-sigchild`
- Add a Sigchild compatibility mode (activated only if PHP has been compiled with it)
This mode is required to use a hack to determine if the process finished with success when PHP has been compiled with the --enable-sigchild option
#5030 will be totally fixed in 2.2 with #5476 as it would allow to send a `SIGKILL` after timeout
---------------------------------------------------------------------------
by stof at 2012-09-18T21:19:50Z
This will also fix the error reported in Behat/MinkZombieDriver#10
The stop method was broken because of the sigchild workaround introduced in the latest 2.1-RC times
Commits
-------
1402b42 Fixing config normalisation example in docblock
Discussion
----------
Fixing config normalisation example in docblock
Against 2.0 this time
as per @stof's comments in symfony/symfony-docs#1721
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: -
Fixes the following tickets: -
Todo: -
License of the code: MIT
