Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Console] Command simplification and deprecations#59564

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
yceruto wants to merge1 commit intosymfony:7.3fromyceruto:command_deprecations

Conversation

yceruto
Copy link
Member

@ycerutoyceruto commentedJan 20, 2025
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?yes
Issues-
LicenseMIT

2 goals in this PR, one feature and one deprecation.

The feature is about delaying the command initialization, making theparent::__construct() call unnecessary in regular/new-invokable commands extending fromCommand class.

Example:

#[AsCommand(name:'foo')]class FooCommandextends Command{publicfunction__construct(privateBarInterface$bar)    {parent::__construct();// <-- unnecessary and can be removed from now on.    }}

Additionally, the static methodsCommand::getDefaultName() andCommand::getDefaultDescription() are deprecated (see discussion#59473 (comment)). These methods are now obsolete becauseAsCommand::name andAsCommand::description are configured directly in the service tag. These methods were mainly intended for framework/DI purposes, making it clearer for end users seeking to set the command name.

@stof
Copy link
Member

Consequently, the constructor method can be removed in Symfony 8.0.

this is not true. Removing the parent constructor would require firstdeprecating calling it (even without a name), so that the code is ready for the removal. And making the parent constructor call mandatory in 6.4 LTS and deprecated in 7.3 would make it harder for the ecosystem to support both versions at once (which might encourage them to drop support for our LTS version before its EOL, which is bad for our communication about the LTS).
The cost of keeping an empty constructor method is null here, so we should give more time to the ecosystem (deprecating calling it in 8.1 for removal in 9.0, allowing to support 7.4 LTS and 8.x with the same code not calling it).

However, I'm not sure encouraging to not call the parent constructor is the right solution.

@ycerutoycerutoforce-pushed thecommand_deprecations branch 3 times, most recently from61c3d4a toa7024e6CompareJanuary 20, 2025 16:42
Copy link
Member

@GromNaNGromNaN left a comment
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Removing the$name parameter from the constructor seems to be an unnecessary breaking change.

@yceruto
Copy link
MemberAuthor

Damn!@stof you're right, I removed that sentence from the PR description. It's hard to deprecate since there is no way to know if the constructor was either called or there is no construct at all in the subclass. Let's revert that part, thanks!

@yceruto
Copy link
MemberAuthor

Update! removed deprecation about passing the command name through the constructor. However, we can update the documentation to remove theparent::__construct() call from all commands.

GromNaN reacted with thumbs up emoji

@yceruto
Copy link
MemberAuthor

I'll split this PR into two, so we can discuss both topics separately

GromNaN reacted with thumbs up emoji

@ondrejmirtes

This comment was marked as off-topic.

@GromNaN

This comment was marked as off-topic.

@yceruto
Copy link
MemberAuthor

moving the deprecation to a separate PR:#59565, and closing the other proposal for now, as it doesn't seem worthwhile at this point

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@GromNaNGromNaNGromNaN left review comments

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@kbondkbondAwaiting requested review from kbond

@welcoMatticwelcoMatticAwaiting requested review from welcoMattic

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

5 participants
@yceruto@stof@ondrejmirtes@GromNaN@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp