[make:subscriber] Improve MakeSubscriber to use KernelEvents constant instead hardcoded event #940
Conversation
jrushlow
force-pushed
the
improveMakeSubscriber.744
branch
from
8710e97
to
1dfe5d1
Compare
There was a problem hiding this comment.
I really like the approach - minor comments, and we need a test!
Also, in the interactive list that we show the users (when asking for which event they want), do we show the string name kernel.controller or the class name - ControllerEvent? We should probably show the class name.
jrushlow
force-pushed
the
improveMakeSubscriber.744
branch
from
1130313
to
37aec4e
Compare
This is what we have - Seems like the best of both worlds |
| $eventName = $eventConstant; | ||
| } else { | ||
| $eventName = class_exists($event) ? sprintf('%s::class', $eventClassName) : sprintf('\'%s\'', $event); | ||
| } |
There was a problem hiding this comment.
I think we could move $eventFullClassName and $eventClassName into this else. And also move the $useStatements->addUseStatement($eventFullClassName); into this as well. It's all just a bit spread out right now.
There was a problem hiding this comment.
I don't think we can without adding more noise.. we have to handle a custom event like \Symfony\MakerBundle\Generator::class and assert that we subscribe to Generator::class => onGenerator. Side note - just added a testcase for this situation
Use KernelEvents constant instead hardcoded event type in MakeSubscriber.
#744