[Documentation] Events Overview Table: Adding "Passed Argument" column

[ThomasLandauer]

As announced in #9160 (comment) I'm adding the passed "EventArgs" class to the overview table. Once this is complete, my further plan is to remove the entire paragraph https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#lifecycle-callbacks-event-argument, and probably also the second code block at https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#entity-listeners-class

@SenseException Is there a better way to link to the source code of LifecycleEventArgs than https://github.com/doctrine/persistence/blob/2.2.x/lib/Doctrine/Persistence/Event/LifecycleEventArgs.php ?
=> Waiting for feedback before I do the others :-)
EDIT: This is an example from Symfony, is this syntax working here too?

:class:Symfony\\Component\\HttpFoundation\\Session\\Attribute\\NamespacedAttributeBag

Also, I changed postLoad to preUpdate in the code block, to have an example that does not receive LifecycleEventArgs ;-)

[ThomasLandauer]

My plan for next steps:

• Move the sections of the single events (starting at https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#prepersist) upwards, directly under the overview table.
• Remove the list at https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#lifecycle-events, by integrating the "remaining" information into the overview table or the respective events' section.

[SenseException]

Also, I changed postLoad to preUpdate in the code block, to have an example that does not receive LifecycleEventArgs ;-)

Remove the list at https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#lifecycle-events, by integrating the "remaining" information into the overview table or the respective events' section.

I haven't checked the text details, but before we can think of removing them as a list, it should be checked if those infos don't contain something important that isn't in the other paragraphs of the events.

[ThomasLandauer]

URGENT: Right now, the PHP code tab seems to be broken: https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#lifecycle-callbacks
So let's get this PR done, so you can merge it!

Why not both?

To keep the code block as short as possible. This is just to show how listeners work in general: One is being shown twice (to showcase that this is possible), plus one another (to showcase that you don't have to always do it twice).

it should be checked if those infos don't contain something important that isn't in the other paragraphs of the events.

Sure, that's exactly what I meant by:

integrating the "remaining" information...

[SenseException]

This PR isn't related to a fix. It also doesn't look like that this has to do with a wrong syntax of events.rst.

[ThomasLandauer]

Is there any real-life proof that .. code-block:: attribute is working at all? That's the main reason why I opened #9161 :-)

[SenseException]

Seems like docs and website are separated. This is going to be fixed in a separate PR.

[ThomasLandauer]

Well, since you've fixed the code-block issue, let's continue here :-)

Remaining question: Is there a better way to link to the source code of LifecycleEventArgs than https://github.com/doctrine/persistence/blob/2.2.x/lib/Doctrine/Persistence/Event/LifecycleEventArgs.php ?

Here's an example from Symfony Docs, is this syntax working here too?

:class:Symfony\Component\HttpFoundation\Session\Attribute\NamespacedAttributeBag

[SenseException]

Here's an example from Symfony Docs, is this syntax working here too?

I'm not fully aware of what the rst-parser is capable of or not. I usually do tests in a local instance of the website. AFAIK this isn't used in the docs yet. There's :php:class: but that doesn't link to a class.

[ThomasLandauer]

Well, so what do you suggest? Should we ask somebody from Symfony?

[SenseException]

The Doctrine website is currently using its own rst-parser, which doesn't support the whole restructured text syntax. For now a link will do because LifecycleEventArgs isn't an ORM class.

[ThomasLandauer]

OK, done :-) Questions:

1. Is https://github.com/doctrine/persistence/blob/master/lib/Doctrine/Persistence/Event/LifecycleEventArgs.php correct at all? Shouldn't this be https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/LifecycleEventArgs.php, like all the others?

2. Which one is correct for preUpdate? https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#entity-listeners-class says PreUpdateEventArgs, but https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#listening-and-subscribing-to-lifecycle-events says LifecycleEventArgs

For the two links to doctrine/persistence, I'm linking to /master/ now, which is being forwarded to /2.2.x/.

[SenseException]

1. Right. It should be consistent. 👍

2. AFAIU is your PR only related to Lifecycle Events. Entity listeners are not part of your table.

[ThomasLandauer]

1. OK, fixed in the table. But to be consistent, it needs to be changed in various other places too, e.g. use in first code block at https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#listening-and-subscribing-to-lifecycle-events Should I do that?
2. PreUpdateEventArgs are mentioned at https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/events.html#preupdate too, so it should be OK.
   In general: To be honest, to straighten out the difference between entity listeners and other listeners (and figuring it out for myself) is one of the reasons I'm doing this :-) So I was hoping, my table comprises entity listeners too?! Is there anything in there yet that's not true for entity listeners?

[SenseException]

Looks nice 👍

So I was hoping, my table comprises entity listeners too?

I wouldn't mix them all together for now.

[ThomasLandauer]

Well, OK, then please merge this :-)

[SenseException]

I'll review the PR soon. Sorry for the delay.

[greg0ire]

Let's use version-independant URLs?

Suggested change

	.. _LifecycleEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/LifecycleEventArgs.php
	.. _PreUpdateEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/PreUpdateEventArgs.php
	.. _PreFlushEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/PreFlushEventArgs.php
	.. _PostFlushEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/PostFlushEventArgs.php
	.. _OnFlushEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/OnFlushEventArgs.php
	.. _OnClearEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/OnClearEventArgs.php
	.. _LoadClassMetadataEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/LoadClassMetadataEventArgs.php
	.. _OnClassMetadataNotFoundEventArgs: https://github.com/doctrine/orm/blob/2.10.x/lib/Doctrine/ORM/Event/OnClassMetadataNotFoundEventArgs.php
	.. _LifecycleEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/LifecycleEventArgs.php
	.. _PreUpdateEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/PreUpdateEventArgs.php
	.. _PreFlushEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/PreFlushEventArgs.php
	.. _PostFlushEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/PostFlushEventArgs.php
	.. _OnFlushEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/OnFlushEventArgs.php
	.. _OnClearEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/OnClearEventArgs.php
	.. _LoadClassMetadataEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/LoadClassMetadataEventArgs.php
	.. _OnClassMetadataNotFoundEventArgs: https://github.com/doctrine/orm/blob/HEAD/lib/Doctrine/ORM/Event/OnClassMetadataNotFoundEventArgs.php

[ThomasLandauer]

Sure, we just didn't know how to do it ;-)

[SenseException]

Easier to maintain, but will at one time link to a different version than the documentation page you're on.

[ThomasLandauer]

Yes.
But: Let's face it - nobody is going to update those links for every new version. So sooner or later they will lead to a different version anyway!
So basically the choice is: to a previous version or to a future version? And since users are upgrading more frequently than downgrading, I'd say: future.

The question isn't too important cause in this case the code isn't changing a lot over the versions (or is it?); and whenever the events are drastically changed, this page needs to be updated anyway.

But I don't have a strong opinion about this, so basically I'm doing what you're telling me :-)

[greg0ire]

HEAD is on the default branch, so that means it would only be an issue if browsing the documentation for the next major, or outdated docs I think.

[SenseException]

Either way, the probability that we'll forget to update the links in both situations are there. 😳
Upmerges will point to the same default branch, but those branches are still in a pre-released state.
Okay, let's keep HEAD.

[greg0ire]

Thanks @ThomasLandauer !

[greg0ire]

@ThomasLandauer hi! The syntax you used for the links in that new column does not appear to work: https://www.doctrine-project.org/projects/doctrine-orm/en/2.11/reference/events.html#events-overview

The leading underscore is not supposed to appear: https://sublime-and-sphinx-guide.readthedocs.io/en/latest/references.html#links-to-external-web-pages

Clicking the "Display the rich diff" button on the "Files changed" tab of Github shows the issue.

Also, the trailing underscore is missing on some entries, which is shows by the syntactic coloration of my editor.

[ThomasLandauer]

@greg0ire Thanks for fixing it!
Since there is no real preview for docs, it's hard to say if something is working...
How often is the website being generated? Would it be possible to make this more obvious? This would be a huge step forward! Right now, I sometimes take a final look at the website a few days after a PR was merged, but sometimes I forget...

[greg0ire]

All we have AFAIK is the "display the rich diff" button, which did show the issue.

I think the website is generated daily: https://github.com/doctrine/doctrine-website/actions

Would it be possible to make this more obvious?

What are you referring to?

[ThomasLandauer]

Rich diff:
Yeah, indeed, in this case. But in the first table column it still just shows the original syntax, not the parsed outcome:

:ref:<reference-events-pre-remove>

So you can never know if the syntax is wrong or just not supported by the rich diff parser...

More obvious:
An automated email to all contributors when their PR goes live. A counter "This PR will go live in xx hours". Or ultimately: Immediate website generation upon each merging of a docs-PR.
Stuff like that is what I had in mind. But since nobody has that, I guess it's just not possible ;-)

[greg0ire]

Your ideas do seem possible actually. Maybe webhooks could help? https://docs.github.com/en/developers/webhooks-and-events/webhooks/about-webhooks