Skip to content

Update event_dispatcher.rst for consistency #7547

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Update event_dispatcher.rst for consistency #7547

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26635bd
Update event_dispatcher.rst for consistency
matthieu88160 Feb 28, 2017
81d5118
Rename event_dispatcher.rst events
matthieu88160 Feb 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/event_dispatcher.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ A call to the dispatcher's ``addListener()`` method associates any valid
PHP callable to an event::

$listener = new AcmeListener();
$dispatcher->addListener('acme.action', array($listener, 'onFooAction'));
$dispatcher->addListener('foo.action', array($listener, 'onFooAction'));
Copy link
Contributor

Choose a reason for hiding this comment

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

What about renaming all of them 'acme.foo.action'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to naming convention described in the previous line of the doc I don't think that naming the event 'acme.foo.action' is the better choice.
Maybe using 'acme_foo.action' can be a prefered approach in regard of these naming convention :

  • "Prefix names with a namespace followed by a dot"
  • "End names with a verb"

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess here foo is a placeholder for acme and action a placeholder for a verb, so maybe acme.foo would be less confusing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the context I agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

By the way the example I given is perfectly valid, see https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/FOSUserEvents.php#L26 as an example :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FOS change password action have some sub-event possibilities, it makes more sense to nest these as you point here than the example show into the event dispatcher one.

To use 'acme.foo.action' I consider using a pattern like '{vendor}.{context}.{action}'. Do you agree with this before I update?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do :)


The ``addListener()`` method takes up to three arguments:

Expand Down