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

Use error style for notifications #10162

Merged
merged 1 commit into from Oct 23, 2022
Merged

Conversation

greg0ire
Copy link
Member

stderr is not a great name for something that is not meant to be processed (piped into) a program, but to be read by humans. Most commands should use stderr, and some of them should partially use stdout, typically when dumping SQL requests.

@greg0ire
Copy link
Member Author

@derrabus doctrine/dbal 3.5.x has been released, which causes Psalm to fail. What should we do about this?

  1. backport fixes to 2.13.x?
  2. release 2.14.x and address the remaining issues?
  3. add the issues to the Psalm baseline on 2.13.x?

@derrabus
Copy link
Member

doctrine/dbal 3.5.x has been released, which causes Psalm to fail.

#10163

@SenseException
Copy link
Member

I don't know why, but 2.14 is still supporting symfony/console 3.0 and SymfonyStyle::getErrorStyle() was introduced in 3.3. As long as we support such old versions the introduced getErrorStyle-calls are breaking changes.

@greg0ire
Copy link
Member Author

greg0ire commented Oct 23, 2022

I'll try to understand why this wasn't caught by the prefer-lowest build, which apparently installs v3.4.17

UPDATE:

composer why-not symfony/console 3.3.0
Info from https://repo.packagist.org: #StandWithUkraine
phpbench/phpbench 1.2.7  requires  symfony/console (^4.2 || ^5.0  || ^6.0)
symfony/yaml      v6.1.6 conflicts symfony/console (<5.4)
vimeo/psalm       4.29.0 requires  symfony/console (^3.4.17 || ^4.1.6 || ^5.0 || ^6.0)

stderr is not a great name for something that is not meant to be
processed (piped into) a program, but to be read by humans.
Most commands should use stderr, and some of them should partially use
stdout, typically when dumping SQL requests.
@greg0ire
Copy link
Member Author

@SenseException I bumped the constraint to 3.3 minimum, that shouldn't be an issue since Symfony v3 is no longer maintained anyway (we could drop support for v3 entirely IMO).

@derrabus
Copy link
Member

Indeed. We still declare compatibility with Symfony Console 3 mainly because we haven't yet used any features that were introduced later and not the other way round. Since Symfony 3 is EOL, we don't need to care about it anymore. So if bumping to 4.4 gets us any benefit, let's do it.

@greg0ire greg0ire merged commit f33919d into doctrine:2.14.x Oct 23, 2022
@greg0ire greg0ire deleted the stderr-for-humans branch October 23, 2022 20:56
@greg0ire greg0ire added this to the 2.14.0 milestone Oct 23, 2022
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

Successfully merging this pull request may close these issues.

None yet

3 participants