-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commands as services #2634
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
Commands as services #2634
Conversation
|
Any clue why Scrutinizer failed? |
b8cb0bc to
7571dfa
Compare
|
@XWB I've just rebased the PR and Scrutinizer has passed |
|
And also I've replaced service id with FQCN |
Resources/config/commands.xml
Outdated
| <service id="FOS\UserBundle\Command\ActivateUserCommand" class="FOS\UserBundle\Command\ActivateUserCommand"> | ||
| <tag name="console.command" /> | ||
| </service> | ||
| <service id="FOS\UserBundle\Command\ChangePasswordCommand" class="FOS\UserBundle\Command\ChangePasswordCommand"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't use FQCN as service ids. We don't want to register these classes in the autowirable types
Resources/config/commands.xml
Outdated
| <tag name="console.command" /> | ||
| </service> | ||
| <service id="FOS\UserBundle\Command\ChangePasswordCommand" class="FOS\UserBundle\Command\ChangePasswordCommand"> | ||
| <tag name="console.command" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add the command attribute with the command name in all tags, so that commands can be lazy-loaded in Symfony 3.4+
7571dfa to
b60b470
Compare
|
@stof Please review |
|
Thanks. |
|
@XWB May we have a new patch release for this? Symfony 4 is stable from a while and commands does not appears with the current stable version of this bundle. |
Prepare the bundle for Symfony 4.0
Relates to #2632