Skip to content
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.

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

Update Entity Listener Docs to New Service Style #1161

Merged
merged 2 commits into from
May 19, 2020

Conversation

huebs
Copy link
Contributor

@huebs huebs commented Apr 30, 2020

Switching to Symfony 4's new container style: classes for service names.

Switching to Symfony 4's new container style: classes for service names.
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 30, 2020

This style is recommended only for applications. For bundles, decoupling the class and the id is still recommended. Also, big -1 as this is a BC break.

@nicolas-grekas
Copy link
Member

Aaaand, this is for the doc, so my comment is void... sorry :D

@SenseException
Copy link
Member

@nicolas-grekas Is this recommendation somewhere in the Symfony docs? Maybe this could be added as a link since the docs just explain how to use the bundle.

Renaming `UserListener` to `App\UserListener` so it is clear to unaware users that it is FCQN

Addressing [note](doctrine#1161 (review)) from @ostrolucky.
Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

This style is recommended only for applications. For bundles, decoupling the class and the id is still recommended.

A link referencing to this information can be added sometime later. For now, this is 👍 . Or do we need to target a different branch and do an upmerge?

@alcaeus alcaeus added this to the 2.1.0 milestone May 19, 2020
@alcaeus alcaeus self-assigned this May 19, 2020
@alcaeus alcaeus merged commit 0ea57dc into doctrine:master May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants