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

Add CLI option to register subscribers #2

Draft
wants to merge 226 commits into
base: event
Choose a base branch
from

Conversation

Jean85
Copy link

@Jean85 Jean85 commented Mar 11, 2020

This is WIP, do not merge.

I was looking into #1 and I would personally like and need a way to add subscribers through CLI options. So I drafted this PR using sebastianbergmann#4136 as a base. While doing this I noticed two different things:

  • there's no way yet to reach the Dispatcher singleton to register new subscribers in any way
  • registering the subscribers in the way that I'm trying to is pretty limited, since it's in the TestRunner: it could happen too late for a number of events that are fired before

So... How do you want to proceed? What steps can I take?

[EDIT] /cc @sebastianbergmann, this is my initial feedback on this implementation

@@ -941,6 +941,10 @@ private function handleConfiguration(array &$arguments): void
$this->addExtension($extension->createHookInstance());
}

foreach ($arguments['subscribers'] as $subscriber) {
// TODO - how to register new subscribers?
Copy link
Owner

Choose a reason for hiding this comment

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

This will be done via our Events\Facade.

Right now, you could query the Facade for the Dispatcher (Events\Facade::dispatcher()) and call register() to add subscribers.

This is work in progress and the Facade will soon provide a direct API without leaking the Dispatcher.

@theseer
Copy link
Owner

theseer commented Mar 11, 2020

Thanks for the work already.

While I'm not yet sure what your goal is, I'm not convinced this is going the right way: A Subscriber is code that needs to be loaded and run. A Subscriber by itself is just a receiver of events. Why would you need a CLI option for that?

We need a technical API to have "things" register subscribers but that step is a prerequisite to actually do something with the event and I don't see how this would be a CLI option?

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