GH-1443: Fix default option when --all-or-nothing option is used as intended
#1444
Conversation
lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php
Outdated
Show resolved
Hide resolved
|
@greg0ire The CI/CD failures do not seem directly related to my changes as far as I could see. Can you take a look if it's something I need to fix in my PR? |
|
You were targeting a branch that is no longer maintained. Please rebase. |
|
Still, the errors occur on 3.8.x, I'll try to address them. |
|
@greg0ire rebasing the branch and the extra commit fixed the CI issues. |
|
Sorry, I forgot to mention I made an attempt to address the SA issues in #1445 |
agustingomes
force-pushed
the
3.7.x
branch
3 times, most recently
from
2fc5665 to
754b035
Compare
docs/en/reference/configuration.rst
Outdated
| @@ -243,18 +243,23 @@ Setting ``transactional`` to ``false`` will disable that. | |||
| From the Command Line | |||
| ~~~~~~~~~~~~~~~~~~~~~ | |||
|
|
|||
| You can also set this option from the command line with the ``migrate`` command and the ``--all-or-nothing`` option: | |||
| To enable all the migrations to complete together, you can also set this option from the command line with | |||
There was a problem hiding this comment.
I'm not sure people will understand what is meant by "completing together". I think we might not want to shy away from being technical here, talking about transactions should make clarify things IMO.
There was a problem hiding this comment.
Pushed a new phrasing, let me know if it's ok the way is written now.
|
The SA issues are fixed, you can now rebase and drop your second commit. |
|
@greg0ire with the release of PHPStan 1.12 today a new error surfaced. How do you want to proceed? |
It affects code that you haven't touched. It needs to be fixed, but not by this PR. See #1453. |
|
@agustingomes you may rebase :) |
These extra tests may help in the future to catch unintended behavior changes for the default of these 2 specific settings.
As result of some past changes to improve the way the option was inferred, several attempts to build a mechanism to handle the absence of value of this option were built via the following commits: 4da00c5 5335951 5038099 799f16d f726a5f This commit leverages all the iterative improvements introduced along the way, and setting the correct value when the option is correctly specified. This should bring the behavior inline with what is currently documented, while additionally adding a documentation deprecation informing users that passing a value to that option won't be allowed in 4.x As an extra addition, this commit also introduces the negated option `--no-all-or-nothing`. The intent is to leverage the native Symfony >= 5.3 [Negatable command options](https://symfony.com/blog/new-in-symfony-5-3-negatable-command-options) in the future once the optional value from `--all-or-nothing` is completely removed.
Summary
After a review of the issue reported (linked above), it seems the default behavior of specifying the option is opposite of what was intended, and also what is currently documented. This PR applies the correct default when the user specifies the
--all-or-nothingoption in the command line.Background:
Previous PR's and their linked issues contain more context regarding each iteration so far. They are presented below in the chronological order in which they were merged:
--all-or-nothingproperly #1296all-or-nothingconsole input value determination. #1308--all-or-nothingwith values #1305