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

$this->eventDispatcher->dispatch(ConfigEvents::DELETE, new ConfigCrudEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::SAVE, new ConfigCrudEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::RENAME, new ConfigRenameEvent($config, $old_name));
$this->eventDispatcher->dispatch(ConfigEvents::IMPORT, new ConfigImporterEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::IMPORT_VALIDATE, new ConfigImporterEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::COLLECTION_INFO, $this->configCollectionInfo);

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

Compact beta evaluation: Unfrozen - API docs only

CommentFileSizeAuthor
#3 2415505-1-configevents-docs.patch4.42 KBeojthebrave

Comments

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Title: Add @Event to all ConfigEvents » Add @Event documentation to all ConfigEvents
eojthebrave’s picture

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

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Status: Active » Needs review
Issue tags: +LatinAmerica2015
StatusFileSize
new 2415505-1-configevents-docs.patch4.42 KB

Here is a first pass at writing the expanded documentation for these events. The RENAME event doesn't have anything in core that is currently subscribed to it other than a test that tests to make sure the event fires/works, so I wasn't able to @see an example implementation for that one.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes

I do not think we use @var on const class members?

+   *
+   * @var string
    */
   const SAVE = 'config.save';

Other than that, this looks good!

Hm. I see that this is all over core. Probably doesn't hurt anything. Let's just go with it.

One other thing though, in the COLLECTION_INFO one:

+   * The event listener method receives a
+   * \Drupal\Core\Config\ConfigCollectionInfo instance.

Is that right? The other ones receive a *Event instance, and this one is not called *Event?

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented

Yeah, that one is correct. I initially was unsure as well. This $this->eventDispatcher->dispatch(ConfigEvents::COLLECTION_INFO, $this->configCollectionInfo); is where it is dispatched, and class ConfigCollectionInfo extends Event so really just an inconsistent name I suppose.

jhodgdon’s picture

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

OK then, patch looks good in that case! Thanks!

alexpott’s picture

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

Committed 370e15e and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed 370e15e on 8.0.x 
    Issue #2415505 by eojthebrave: Add @Event documentation to all...

Status: Fixed » Closed (fixed)

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