Fix --complete flag in orm:ensure-production-settings command #8426
Conversation
| @@ -62,7 +62,7 @@ protected function execute(InputInterface $input, OutputInterface $output) | |||
| try { | |||
| $em->getConfiguration()->ensureProductionSettings(); | |||
|
|
|||
| if ($input->getOption('complete') !== null) { | |||
| if (!$input->getOption('complete')) { | |||
There was a problem hiding this comment.
current symfony returns false not sure if null was returned in older version so I did switch to using ! instead for checking for false or null.
There was a problem hiding this comment.
There is a default value that can be set in addOption. Without an explicit value it is null.
https://github.com/symfony/console/blob/5.x/Command/Command.php#L400
It seems to be the same in 4.0 and 3.0.
There was a problem hiding this comment.
Seems like its not possible to set a default value for InputOption::VALUE_NONE: https://github.com/symfony/console/blob/356c70659d6b48595f9f275cd7c4de0d5544c49c/Input/InputOption.php#L168
There was a problem hiding this comment.
So I could go with:
$input->getOption('complete') === true
$input->getOption('complete') !== false
or the current version.
The value of the input is set here to true:
https://github.com/symfony/console/blob/356c70659d6b48595f9f275cd7c4de0d5544c49c/Input/ArgvInput.php#L248
And default value to false:
There was a problem hiding this comment.
If it's a boolean then your current syntax should fit. You currently let it step into the if-block when the value is false. It also looks like there are no tests yet that covers this class. Can you please create one that covers your changes?
There was a problem hiding this comment.
@SenseException created the missing test class hope it matches how tests are created in this repository.
alexander-schranz
force-pushed
the
patch-1
branch
from
eff92c6 to
c6f5320
Compare
alexander-schranz
force-pushed
the
patch-1
branch
from
c6f5320 to
0025331
Compare
|
@doctrine/team-orm Do we need |
|
I don't think so. |
|
Change to the DoctrineTestCase |
|
Anything I can do to get this merged? |
|
Currently not. This will be handled by another reviewer after a second pair of eyes take a look at this PR. |
ostrolucky
changed the title
$input->getOption('complete') returns
falseand notnullif not set.