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 support trigger($eventName) and Cake\EventManager proxy #119

Closed
wants to merge 4 commits into from

Conversation

samsonasik
Copy link
Member

@gianarb this is to prove #118 and added Cake\EventManager proxy to prove ;)

@samsonasik samsonasik mentioned this pull request Nov 15, 2015
@gianarb
Copy link
Contributor

gianarb commented Nov 16, 2015

Thanks
Why have you drop the PennyEventManagerInterface? :)

This interface define a compatible event

@samsonasik
Copy link
Member Author

because it make hard to implement proxy for other Event Manager, it will get error like: Cakephp\Event\EventManager dispatch must pass PennyEventManagerInterface, Cakephp\Event\Event passed. We need to create a driver class that implements PennyEventManagerInterface just to make a proxy works, Also, provide a string event as key/event name is more usable imo.

@gianarb
Copy link
Contributor

gianarb commented Nov 16, 2015

We need to create a driver class that implements PennyEventManagerInterface just to make a proxy works

Yes without this step your events could not support setRequest or setResponse or setRouteInfo..
penny uses this methods https://github.com/pennyphp/penny/blob/master/src/App.php#L94
I understand that an unused interface is a bad practice.. But maybe this interface is helpful

Maybe we rename (I follow your old feedback) this event_manager in penny.event_manager.. :) The penny.event_managermanages only application flow.. :)

@samsonasik
Copy link
Member Author

Ok, I understand, I will try to create separate PR for alternative that uses PennyEventInterface and proxy sample for cakephp.

Warm regards,

Abdul Malik Ikhsan

Pada 16 Nov 2015, pukul 15.32, Gianluca Arbezzano notifications@github.com menulis:

We need to create a driver class that implements PennyEventManagerInterface just to make a proxy works

Yes without this step your events could not support setRequest or setResponse or setRouteInfo.. And penny uses this methods https://github.com/pennyphp/penny/blob/master/src/App.php#L94


Reply to this email directly or view it on GitHub.

@gianarb
Copy link
Contributor

gianarb commented Nov 16, 2015

Thanks :)

@samsonasik
Copy link
Member Author

closing in favor of #120

@samsonasik samsonasik closed this Nov 16, 2015
@samsonasik
Copy link
Member Author

@gianarb re-open, probably add another method: setupEvent(PennyInterface $event) for set event must be instance of PennyEventInterface ?

@samsonasik samsonasik reopened this Nov 16, 2015
@samsonasik
Copy link
Member Author

re-closing, seems not possible :p

@samsonasik samsonasik closed this Nov 16, 2015
@samsonasik
Copy link
Member Author

Hm...., how about add triggerByEventName() ?

Warm regards,

Abdul Malik Ikhsan

Pada 16 Nov 2015, pukul 15.56, Gianluca Arbezzano notifications@github.com menulis:

Thanks :)


Reply to this email directly or view it on GitHub.

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.

2 participants