This issue is for adding the @Event tag and appropriate documentation to the following list of events.

$this->dispatcher->dispatch(RoutingEvents::DYNAMIC, new RouteBuildEvent($collection));
$this->dispatcher->dispatch(RoutingEvents::ALTER, new RouteBuildEvent($collection));
$this->dispatcher->dispatch(RoutingEvents::FINISHED, new Event());

This documentation should include the following information:

- What the event is
- What it is used for
- What kind of $event object is passed along.
- @see links to related classes (event class, representative class where event is dispatched, representative class where event is subscribed to).

See the parent issue for a documentation template, and more information: #2382169: [meta] Add @Event to all events defined by drupal core

Beta eval: This is API documentation, so unfrozen. Also very low disruption possibility.

CommentFileSizeAuthor
#9 2415513-9-routingevents-docs.patch2.73 KBeojthebrave
#4 2415513-4-routingevents-docs.patch2.72 KBeojthebrave
#1 2415513-1-routingevents-docs.patch2.65 KBeojthebrave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
Status: Active » Needs review
FileSize
2415513-1-routingevents-docs.patch2.65 KB

Here's a first pass at documentation for these events.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
Issue summary: View changes

Adding beta-eval info to issue summary, as per other similar issues.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Thanks for the patch!

Do you think commas would be useful before "giving" in this paragraph and the other similar one? Otherwise the sentence seems a bit hard to parse:

+   * This event is used to add new routes based upon existing routes giving
+   * modules the opportunity to dynamically generate additional routes. The
+   * event listener method receives a \Drupal\Core\Routing\RouteBuildEvent
+   * instance.

I'm also wondering if we should change the first lines:

+   * The DYNAMIC event is fired on a route collection to allow new routes.

The other @Event blocks we've defined in other patches on the parent meta-issue are using style like:
The name of the event ...
I'd vote for consistency.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
Status: Needs work » Needs review
Issue tags: +LatinAmerica2015
FileSize
2415513-4-routingevents-docs.patch2.72 KB

- Added commas to fix some run-on sentences.
- Change the first sentence of documentation for each event to make them consistent with other already documented events

Ready for another review.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
Issue tags: +Novice
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community

Looks good now, thanks!

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/lib/Drupal/Core/Routing/RoutingEvents.php
@@ -13,21 +13,55 @@
+   * building has completed. The event listener receives an
+   * \Drupal\Core\Routing\RouteBuildEvent instance.

Note: Its just an Event, because by definition we construction we did not want to pass the routes along. This is at least the state in head:

    $this->dispatcher->dispatch(RoutingEvents::FINISHED, new Event());
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Reviewed & tested by the community » Needs work

Sounds like we need to update the documentation then?

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
Status: Needs work » Needs review
FileSize
2415513-9-routingevents-docs.patch2.73 KB

Oops. Here's an updated version with `\Symfony\Component\EventDispatcher\Event` reference for the FINISHED event.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community

Looks good! I double-checked the other two and they're passed the event classes in the docs.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 6dc2481 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed 6dc2481 on 8.0.x 
    Issue #2415513 by eojthebrave: Add @Event documentation to all...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.