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

Fix deprecated $defaultName #278

Closed
wants to merge 1 commit into from
Closed

Fix deprecated $defaultName #278

wants to merge 1 commit into from

Conversation

ruudk
Copy link

@ruudk ruudk commented May 27, 2022

This fixes a deprecation on Symfony 6.1:
The "Symfony\Component\Console\Command\Command::$defaultName" property is considered final. You should not override it in "Ekino\NewRelicBundle\Command\NotifyDeploymentCommand".

We cannot use #[AsCommand] as that will still complain about the $defaultName being overwritten... and we have to support SF 3.4.

This fixes a deprecation on Symfony 6.1:
The "Symfony\Component\Console\Command\Command::$defaultName" property is considered final. You should not override it in "Ekino\NewRelicBundle\Command\NotifyDeploymentCommand".
@ruudk ruudk changed the title Override getDefaultName to fix deprecation on SF 6.1 Fix deprecated $defaultName May 27, 2022
@ruudk
Copy link
Author

ruudk commented Jun 9, 2022

@jderusse Can this be merged please?

@jderusse
Copy link
Collaborator

jderusse commented Jun 9, 2022

CI needs to be fixed first to assert your PR pass the testsuite. Would you mind to open a PR to downgrade composer in PHP 7.1

@ruudk
Copy link
Author

ruudk commented Jun 9, 2022

Not going to do that. PHP 7.1 is EOL for years.

I would be able to create a PR to drop all EOL PHP versions.

@CRC-Mismatch
Copy link

CRC-Mismatch commented Jun 10, 2022

Easiest bet is my PR #281, it has fixed CI and all, but it's either restricting support for newer Monolog versions or dropping 7.1 🤷‍♂️
I mean, even composer itself needs a whole workaround to work in CI with 7.1, as of now...

@ruudk
Copy link
Author

ruudk commented Jun 13, 2022

Decided that the best thing to do is to remove this bundle + dependency completely from our project.

It was added in the project years ago, a time where "there is an app a bundle for that™️" was the way to go. But we've learned our lesson, dependencies should not be added that easily.

@ruudk ruudk closed this Jun 13, 2022
@ruudk ruudk deleted the patch-1 branch June 13, 2022 11:31
@ruudk ruudk restored the patch-1 branch August 17, 2023 06:47
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.

3 participants