Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Added Doctrine entities and documents to the list of known locations for classes#7156
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
wouterj commentedNov 22, 2016
👍 |
bundles/best_practices.rst Outdated
| Event Listeners ``EventListener/`` No | ||
| Configuration ``Resources/config/`` No | ||
| Web Resources (CSS, JS, images) ``Resources/public/`` Yes | ||
| Translation files ``Resources/translations/`` Yes |
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.
To complete the PR, WDYT about adding/Resources/config/validation and/Resources/config/serialization too?
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'm not sure because those directories are optional when using annotations. In this case, I'd also like to have a dedicated article to explain these things, as stated in a previous comment.
bundles/best_practices.rst Outdated
| Commands ``Command/`` Yes | ||
| Controllers ``Controller/`` No | ||
| Service Container Extensions ``DependencyInjection/`` Yes | ||
| Doctrine entities and documents ``Entity/`` (ORM) or ``Document/`` (ODM) No |
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.
Why did you tag this as not mandatory?
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.
Because thanks to thedoctrine.orm.mapings you can store your entities anywhere you like, even outside the bundles.
In any case, I dislike thisMandatory column more and more. The intention is good, but this cannot simply be answered by Yes or No. I propose to remove this column and add a new article explaining how to override the location every single part of the bundle: controllers, templates, entities, commands, etc.
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 same argument applies to nearly all other rows where have set this column to "yes". So I would rather say let's remove the column.
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.
By the way, we have such an article:http://symfony.com/doc/current/bundles/override.html
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.
👍 for removing this column.
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.
see also#8269 which made some changes in the3.4 branch because auto-discovering commands based on their location will be deprecated
…n locations for classes (javiereguiluz)This PR was squashed before being merged into the 2.7 branch (closes#7156).Discussion----------Added Doctrine entities and documents to the list of known locations for classesCommits-------8764708 Final updates469d0ea Added Doctrine entities and documents to the list of known locations for classes
No description provided.