Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
tip for mapping definition#5011
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
If you follow the link from the security documentation (http://symfony.com/doc/current/book/security.html#loading-users-from-the-database), it's easy to miss the necessary mapping definition for the repository.
i am not sure if we really need this hint but i am 👍 with this PR |
It's a common pitfall with newbies at our company ;) |
I see how this can be missed if you aren't very used to Doctrine. 👍 for adding this tip |
| .. tip:: | ||
| Don't forget to | ||
| :ref: `add the repository class to the mapping definition of your entity <book-doctrine-custom-repository-classes>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The whole block should be indented by four spaces. And can we maybe shorten theref label a bit (maybe using "of your entity" out of it)? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Shorten the ref? Good idea. But how can i change the indentation now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ah, never mind. Found the edit button : P
| .. tip:: | ||
| Don't forget to add the repository class to the mapping definition of | ||
| :ref: ` your entity <book-doctrine-custom-repository-classes>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think I would change it to the following:
..tip:: Don't forget to add the repository class to the:ref:`mapping definition of your entity<book-doctrine-custom-repository-classes>`.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I like it
Thanks for the fast reactions@SrgSteak. It reads really well. 👍 |
| @@ -583,6 +583,11 @@ The code below shows the implementation of the | |||
| } | |||
| } | |||
| .. tip:: | |||
| Don't forget to add the repository class to the | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
perhaps here there is no cause or reasoning as to why we shouldn't forget. Maybe explain don't forget else it will not be hooked or it will just use the default class or other worst.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think it's needed to make this more verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree with@wouterj. I think the "why" would go beyond the scope of this tip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Agreed, anyone interested in the background will find the needed information in the referenced chapter.
This PR was merged into the 2.3 branch.Discussion----------tip for mapping definitionIf you follow the link from the security documentation (http://symfony.com/doc/current/book/security.html#loading-users-from-the-database), it's easy to miss the necessary mapping definition for the repository.Commits-------7ae3a28 improved link38dcfc8 fixed intendation and shortened the link4422c10 tip for mapping definition
Missed one of my tweaks (didn't save!). See sha:8119e85. The extra space was why the build wasn't originally failing :) |
* 2.3: [#5015] Very small tweak [#5011] Adding one more fix I missed [#5011] Fixing minor build issue improved link fixed intendation and shortened the link Adding a break statement to improve the sample code Added an example about how to get the impersonating user object#4032 improved comments about em option tip for mapping definition
* 2.6: (91 commits) [#5064] Minor language tweaks Fixing bad merge conflict (forgot to save!) Remove unnecessary component reference Correct RegisterListenersPass namespace Fix service id Switched the first example to a static constructor method added some more components for Tobion as a merger Fixed variable name in : Reference -> validation constraints -> count -> basic usage -> PHP [#5036] Typo fix (probably mine originally) caught by xabbuh reword to serves Adding a link to define "lts" Better wording Minor improvement for symfony-installer with LTS Updating for new security service names in 2.6 [#5033] Tweaking variable name to "match" the service id [#5017] Minor language tweaks [#5015] Updating the security service name for 2.6 - thanks to Cordoval [#5015] Very small tweak [#5011] Adding one more fix I missed [#5011] Fixing minor build issue ...Conflicts:book/security.rst
* 2.7: (103 commits) Backporting some stuff from 2.7, that I think must have gotten merged only there by accident [#5064] Minor language tweaks Fixing bad merge conflict (forgot to save!) Remove unnecessary component reference Correct RegisterListenersPass namespace Fix service id Switched the first example to a static constructor method added some more components for Tobion as a merger Fixed variable name in : Reference -> validation constraints -> count -> basic usage -> PHP [#5036] Typo fix (probably mine originally) caught by xabbuh reword to serves Adding a link to define "lts" Better wording Minor improvement for symfony-installer with LTS Updating for new security service names in 2.6 [#5033] Tweaking variable name to "match" the service id [#5017] Minor language tweaks [#5015] Updating the security service name for 2.6 - thanks to Cordoval [#5015] Very small tweak [#5011] Adding one more fix I missed ...
Nice catch@weaverryan! :) |
If you follow the link from the security documentation (http://symfony.com/doc/current/book/security.html#loading-users-from-the-database), it's easy to miss the necessary mapping definition for the repository.