-
-
Notifications
You must be signed in to change notification settings - Fork 437
[make:listener] Add global command for listener and subscriber #1366
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
[make:listener] Add global command for listener and subscriber #1366
Conversation
weaverryan
left a comment
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.
This is a cool idea 👍
| if (method_exists($class, 'getCommandAlias')) { | ||
| $alias = $class::getCommandAlias(); | ||
| $commandDefinition->addTag('console.command', ['command' => $alias, 'description' => 'Deprecated alias of "make:event"']); | ||
| } |
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.
Should/could we leverage the getCommandAliases() referenced above since that logic already exists?
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.
+1, I found it a little bit weird to have methods like getCommandAliases whose existence leads to messages for very specific classes.
Maybe something like this?
if (method_exists($class, 'getDeprecatedCommandAliases')) {
foreach ($class::getDeprecatedCommandAliases() as $alias) {
$commandDefinition->addTag('console.command', ['command' => $alias, 'description' => \sprintf('Deprecated alias of "%s"', $class::getCommandName())]);
}
}|
@weaverryan Command renamed To resume: If Listener or Subscriber was found in the suffix we display the originals events to choose, as the legacy command did. And if no name was filled, a question is displayed: I didn't really understand your comment, if I was right to notify a deprecation about
If it good for you, maybe we would need to create a new Issue for the next major version to clean all of this deprecation as:
PS: not related to this PR but we also have deprecated code about |
It looks correct what you did 👍
Yes - though no issue needed, this will naturally happen. I don't know when we might release a new major version, but the main thing we will do when that happens is hunt down all of the deprecated code paths and remove them. Assuming we've done a good job marking this with |
weaverryan
left a comment
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.
Don't forget to check the php-cs CI job for some coding standards stuff :)
|
Need help @weaverryan , I don't understand why it failed in the appveyor. It only failed when Listener are generated. But in local or during the other tests in the CI it work. By the way I updated the deprecated messages and finally removed it from MakerExtension.php and added it in the maker.xml |
0307eac to
1bda950
Compare
|
Thank you Steven! |

Related to #1301 I also suggest to add a global maker for both, Listener and Subscriber.
I worked with the based class
MakeSubscriberand updated it to make this solution.In the terminal you can run
make:eventand a question will be displayed like that to know which event class you want to create.And if you write well what you want as
FooBarSubscriberorFooBarListenerand the event to listen, your file will be generated as Listener or Subscriber.