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.

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

Deprecation always triggers with or without --all-or-nothing option #1379

Closed
endelwar opened this issue Nov 28, 2023 · 7 comments
Closed

Deprecation always triggers with or without --all-or-nothing option #1379

endelwar opened this issue Nov 28, 2023 · 7 comments

Comments

@endelwar
Copy link

Bug Report

Q A
BC Break no
Version 3.7.1

Summary

When running bin/console doctrine:migrations:migrate deprecation for passing values to --all-or-nothing always triggers because option has default value.

Current behavior

Due to #1296 a default option for --all-or-nothing is always set ('notprovided'), making check on

$wasOptionExplicitlyPassed = $input->hasOption('all-or-nothing');
to always set $allOrNothingOption to a not null value, which then triggers the deprecation:

User Deprecated: 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 (ConsoleInputMigratorConfigurationFactory.php:44 called by ConsoleInputMigratorConfigurationFactory.php:22, https://github.com/doctrine/migrations/issues/1304, package doctrine/migrations) {"exception":"[object] (ErrorException(code: 0): User Deprecated: Context: Passing values to option `--all-or-nothing`\nProblem: Passing values is deprecated\nSolution: If you need to disable the behavior, omit the option,\notherwise, pass the option without a value (ConsoleInputMigratorConfigurationFactory.php:44 called by ConsoleInputMigratorConfigurationFactory.php:22, https://github.com/doctrine/migrations/issues/1304, package doctrine/migrations) at /Users/manuel/Sites/myproject/vendor/doctrine/deprecations/lib/Doctrine/Deprecations/Deprecation.php:210)

How to reproduce

Run bin/console doctrine:migrations:migrate and look at the logs

Expected behavior

I expect no deprecation triggered when not setting the --all-or-nothing option

Proposed solution

Remove the default value of all-or-nothing option in Doctrine\Migrations\Tools\Console\Command\MigrateCommand::configure

@greg0ire
Copy link
Member

Cc @agustingomes

@greg0ire
Copy link
Member

Proposed solution

Remove the default value of all-or-nothing option in Doctrine\Migrations\Tools\Console\Command\MigrateCommand::configure

Why not instead alter the computation of $wasOptionExplicitlyPassed to see check if the value is notprovided?

@stof
Copy link
Member

stof commented Nov 28, 2023

$input->hasOption('all-or-nothing') does not check at all whether this option was passed. It checks whether this option is defined.

@gaetan-petit
Copy link

Isn't as easy as changing notprovided to null in Doctrine\Migrations\Tools\Console\Command\MigrateCommand::configure
Or even to remove the default value?

@agustingomes
Copy link
Contributor

@gaetan-petit From my understanding, is not as easy as what you mention because of what @stof mentioned. That complicates the check a bit, as you can see in the PR you have opened with the test case that is failing.

This is what I wrote back when I did the commit adding the notprovided:

This test case addition simulates what happens when the `--all-or-nothing` option is indicated, but no explicit value is passed.

We'll set a default option, which is retrieved in case the option has not been provided, allowing us to override the value in case the option is different than the default value.

agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Nov 30, 2023
Unfortunately as part of the previous improvement to determine whether a deprecation is thrown, validating that the deprecation was not thrown ended up not being added explicitly.

In addition, adds the explicit expectation of not throwing the deprecation whenever the `--all-or-nothing` is not indicated, which was the root cause originating the issue.
agustingomes added a commit to agustingomes/doctrine-migrations that referenced this issue Nov 30, 2023
Unfortunately as part of the previous improvement to determine whether a deprecation is thrown, validating that the deprecation was not thrown ended up not being added explicitly.

In addition, adds the explicit expectation of not throwing the deprecation whenever the `--all-or-nothing` is not indicated, which was the root cause originating the issue.
greg0ire added a commit that referenced this issue Dec 5, 2023
…n-expectation-check

GH-1379: Improve Deprecation thrown check + logic
@agustingomes
Copy link
Contributor

@endelwar there's a new release available which should have this issue fixed.

@greg0ire greg0ire closed this as completed Dec 5, 2023
@endelwar
Copy link
Author

endelwar commented Dec 6, 2023

Great work! thanks!

derrabus added a commit to derrabus/migrations that referenced this issue Mar 1, 2024
* 3.8.x:
  Flatten directory tree
  Switch to attributes in docs (doctrine#1392)
  chore: Add missing option in MigrateCommand help section
  ci: fix variable reference (doctrine#1385)
  Fix formatParameter() for boolean (doctrine#1377)
  doctrineGH-1379: Improve Deprecation thrown check + logic
  Updated documentation for config-cli.php file according to v3.7.x
derrabus added a commit to derrabus/migrations that referenced this issue Mar 1, 2024
* 3.8.x:
  Adjust PHPStan settings for ORM 3 and DBAL 4 (doctrine#1404)
  Flatten directory tree
  Switch to attributes in docs (doctrine#1392)
  chore: Add missing option in MigrateCommand help section
  ci: fix variable reference (doctrine#1385)
  Fix formatParameter() for boolean (doctrine#1377)
  doctrineGH-1379: Improve Deprecation thrown check + logic
  Updated documentation for config-cli.php file according to v3.7.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants