Problem/Motivation
In #2264049: Create an Events topic we provided a solution to document events, more background there.
One example would be
/**
* Denotes an event at the very beginning of request dispatching.
*
* This event allows you to create a response for a request before any
* other code in the framework is executed. The event listener method
* receives a \Symfony\Component\HttpKernel\Event\GetResponseEvent
* instance.
*
* @Event
*
* @var string
*/
const REQUEST = 'kernel.request';
Beta phase evaluation
| Issue category | Task because it is not a bug (doesn't effect functionality), and not a feature request. |
|---|---|
| Unfrozen changes | Unfrozen because it only changes documentation |
This is allowed because documentation changes are unfrozen. (Does not need disruption evaluated.)
Proposed resolution
- Find all events in Drupal core, all classes with the name *Events, probably
- Add the @Event annotation to all of them.
- On top of that describe what kind of $event object is passed in, see previous example.
Remaining tasks
| Task | Novice task? | Contributor instructions | Complete? |
|---|---|---|---|
| Agree on formatting of the example |
User interface changes
No.
API changes
No.
Comments
Comment #1
dawehnerA rough example can be found on https://www.drupal.org/files/issues/2264049-27.patch
Comment #2
dawehnerComment #3
jhodgdonIn the issue summary for the other issue, I proposed that the documentation should include:
- 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).
Let's treat this as the "gold standard" for great event docs; at a minimum I think we want to make sure we have the first three items documented.
As a note, the example in the issue summary is maybe not great. It seems like the first-line summary should probably be standardized... maybe something like:
Indicates a termination event, which is triggered when ...
???
Comment #4
dawehnerWell, this is an example of symfony, which we obviously can't "fix", but to be honest this is a really uncommon example (not used by Drupal).
Updated the IS with another example.
Comment #5
jhodgdonHm. The suggested doc block needs a one-line summary... and normally one-line summaries are not declarative sentences like "The foobar function does this" or "The FOOBAR constant is this", but more like "Calculates the foo of the bar" for functions, and "Indicates that foo is very bar-like" for constants.
So again, I'd suggest we adopt a standardized one-line summary, something more like:
Denotes an event at the very beginning of request dispatching.
or something like that. (Indicates? Encodes? Denotes? I am not sure what verb to use.)
Also any mention of a class in a doc block should start with a fully-qualified namespace, so \Symfony\... (start with backslash).
And we do not use @api in our docs.
Comment #6
yesct commentedadded beta evaluation to summary.
Comment #7
yesct commentedmoved summary reason outside the table so it showed after the table.
Comment #8
jhodgdonI like the example currently in the issue summary.
Comment #9
dawehnerSorry totally forgot about this issue ...
Great, does that mean we can work on the policy? See my latest comment on the critical one: #2264049-38: Create an Events topic
Comment #10
eojthebraveHere's a list of the events I could find dispatched by some Drupal component or another. Based on searching for uses of EventDispatcher::dispatch(). There may be others but this is what I could find.
Should we open separate tickets for each set of events so that it's easier to review, or just do them all in this ticket?
Comment #11
jhodgdonI think we should make separate issues for the different modules/components.
I don't see the problem with the block event. It looks like the first argument to dispatch() is still a constant?
And by the way, the API module is ready to display @Event on api.drupal.org on the new Events topic... once we deploy a new version to api.drupal.org anyway, which I'll probably make happen in the next two weeks. Hooray! It already works on my test site with the Symfony events, which already have @Event in them.
Comment #12
jhodgdonUpdate: as the Symfony events already have @Event on them, and the API module is deployed, they are now being displayed on api.drupal.org here:
https://api.drupal.org/api/drupal/core!modules!system!core.api.php/group...
The sooner we can get this done with our event constants, the better!
Comment #13
eojthebraveI created new issues for each of the event "groups", and made this issue the parent for each of them.
- #2415505: Add @Event documentation to all ConfigEvents
- #2415507: Add @Event documentation to all FieldStorageDefinitionEvents
- #2415511: Add @Event documentation to all RenderEvents
- #2415513: Add @Event documentation to all RoutingEvents
- #2415515: Add @Event documentation to all BlockEvents
- #2415519: Add @Event documentation to all LanguageConfigOverrideEvents
Comment #14
jhodgdonPerfect, thanks!
Comment #15
jhodgdonComment #16
eojthebraveI think we can close this now. All of the child issues are resolved.
I also updated https://www.drupal.org/coding-standards/docs with information about the
@Eventtag since that didn't get done in #2264049: Create an Events topic, though that got closed already.Comment #17
jhodgdonGreat! Thanks for all the patches too!
Comment #18
jhodgdonComment #19
dawehnerGreat work!
Comment #20
jhodgdonhttps://api.drupal.org/api/drupal/core!modules!system!core.api.php/group...
!!!
Comment #21
webchickSo. AWESOME. Thanks so much, folks!!
Comment #22
xanoSemi-follow-up: #2443763: Expose events as plugins.