Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

--all-or-nothing without value computes to false #1443

Closed
MatTheCat opened this issue Aug 23, 2024 · 6 comments
Closed

--all-or-nothing without value computes to false #1443

MatTheCat opened this issue Aug 23, 2024 · 6 comments

Comments

@MatTheCat
Copy link

MatTheCat commented Aug 23, 2024 •
edited
Loading

Bug Report

Q A
BC Break not sure
Version 3.7.4

Summary

There seems to be a discrepancy between this deprecation:

migrations/src/Tools/Console/ConsoleInputMigratorConfigurationFactory.php

Lines 46 to 49 in 535a70d

Context: Passing values to option `--all-or-nothing`
Problem: Passing values is deprecated
Solution: If you need to disable the behavior, omit the option,
otherwise, pass the option without a value

and this test:

migrations/tests/Tools/Console/Command/MigrateCommandTest.php

Line 384 in 535a70d

yield [false, ['--all-or-nothing' => null], false, false];

which asserts that --all-or-nothing without value will compute to false.

Current behavior

Running a migration with the --all-or-nothing option without value, “all or nothing” mode is disabled (whatever the value from the configuration).

How to reproduce

Run a migration passing the --all-or-nothing option without value.

Expected behavior

“All or nothing” mode is enabled.
@greg0ire
Copy link
Member

@agustingomes can you please look into it?

@agustingomes
Copy link
Contributor

@greg0ire I'll try during the weekend. Likely is the deprecation text that should be adjusted.

@MatTheCat
Copy link
Author

MatTheCat commented Aug 23, 2024 •
edited
Loading

Note that it seems the current behavior doesn’t match the documentation either:

You can also set this option from the command line with the migrate command and the --all-or-nothing option:

./vendor/bin/doctrine-migrations migrate --all-or-nothing

agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 23, 2024
@agustingomes
doctrineGH-1443: bring --all-or-nothing option in line with documen…
cd7014d 
…ted behavior

As result of some past changes to deal with the confusing way the option was inferred via an upstream package, 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
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 23, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
eeb16a3 
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
@agustingomes agustingomes mentioned this issue Aug 23, 2024
Merged
@agustingomes
Copy link
Contributor

@MatTheCat @greg0ire I created a PR with an initial iteration of the possible fix: #1444

Let's discuss in the PR how to move forward.

agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 25, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
8eefab0 
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
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 25, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
0532c43 
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.
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 25, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
2fc5665 
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.
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 25, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
754b035 
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.
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 27, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
eac359a 
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.
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 27, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
d186b4a 
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.
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 28, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
ddddf6f 
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.
greg0ire added a commit that referenced this issue Aug 28, 2024
@greg0ire
Merge pull request #1444 from agustingomes/3.7.x
7760fbd 
GH-1443: Fix default option when `--all-or-nothing` option is used as intended
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Aug 28, 2024
@agustingomes
doctrineGH-1443: fix --all-or-nothing default behavior
b6ee111 
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.
@agustingomes
Copy link
Contributor

I believe this issue may now be closed with the new 3.8.1 release

https://github.com/doctrine/migrations/releases/tag/3.8.1

@MatTheCat
Copy link
Author

That was fast! Thanks ♥️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@greg0ire @MatTheCat @agustingomes