-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Added a cookbook section about event subscribers #5377
Conversation
How to Create an Event Listener | ||
=============================== | ||
How to Create Event Listeners and Subscribers | ||
============================================= | ||
|
||
Symfony has various events and hooks that can be used to trigger custom |
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.
[...] to perform custom [...]
|
||
.. code-block:: yaml | ||
|
||
# app/config/config.yml |
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 be services.yml
here too (same below for the other formats).
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.
Fixed. Thanks.
All issues have been fixed. Can we label this as |
Symfony has various events and hooks that can be used to perform custom | ||
actions in your application. Those events are triggered by the HttpKernel | ||
component and they are defined in the :class:`Symfony\\Component\\HttpKernel\\KernelEvents` | ||
class. |
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.
Imo this paragraph contradicts a bit your other PR where you try to clarify that user's don't have to provider their own event dispatcher for custom events.
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.
I'm stuck at this. Can someone please provide me an alternative to this paragraph? Thanks.
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.
IMHO this paragraph is ok. I don't see any problem here.
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.
Probably, adding a short note after this paragraph that you can dispatch your own events using the core event dispatcher and that there are lots of more events provided by third-party libraries, could be sufficient (we can link to related articles).
I've updated this PR to add a note comparing listeners and subscribers, as asked in #3987. |
Please, tell me what else I can do to get this PR labelled as |
|
||
</service> | ||
</services> | ||
</container> |
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.
<!-- app/config/services.xml -->
<?xml version="1.0" encoding="UTF-8" ?>
<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="app.exception_subscriber"
class="AppBundle\Subscriber\ExceptionSubscriber">
<tag name="kernel.event_subscriber"/>
</service>
</services>
</container>
the PR title is weird. this would be better IMO: |
e07cbcb
to
d92791e
Compare
I've reworded the introduction and also made the required changes. This is ready for the final review. Thanks! |
Conflicts: cookbook/service_container/event_listener.rst
d92791e
to
ba9ec6b
Compare
In http://symfony.com/doc/current/cookbook/bundles/best_practices.html, directory for events listeners is |
Indeed, can you make that last change, Javier? Then, this is ready to be merged. |
I've made some final changes in b5a82ca. |
👍 Thanks Javier. That's a really nice work and I think we are finally finished here. :) |
Thanks Javier - and sorry for the extreme delay - I personally am catching up after some time away :) |
…888, javiereguiluz) This PR was merged into the 2.3 branch. Discussion ---------- Added a cookbook section about event subscribers | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes | Applies to | all | Fixed tickets | - This PR finishes the work made by @beni0888 in #4538. Commits ------- b5a82ca Final changes ba9ec6b Reworded the introduction and other minor fixes 483f029 Added a note about the advantages/drawbacks of listeners/subscribers e56fed8 Fixed minor issues c8c8bf8 Reworded the subscriber introduction 36b1d10 Fixed the name of the services file a444951 Implemented the suggestions made by @xabbuh 9a6dab7 Completed the cookbook about the event subscriber 0184e0f Added a note about the priority meaning in event subscribers
@javiereguiluz During merging this up the branches, something seems to have been go wrong. I fixed the build in #5792. Would you mind checking if everything you added here is still present and correct? |
I've checked and, at least in 2.3 branch, the article contains all the proposed changes, so nothing got lost. |
@javiereguiluz It was broken on the |
This PR finishes the work made by @beni0888 in #4538.