Skip to content

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 2, 2017
Merged

Added Doctrine entities and documents to the list of known locations for classes #7156

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
469d0ea
Added Doctrine entities and documents to the list of known locations …
javiereguiluz Nov 22, 2016
8764708
Final updates
javiereguiluz Oct 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions bundles/best_practices.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,23 +116,20 @@ files are going to be part of the repository.
The following classes and files have specific emplacements (some are mandatory
and others are just conventions followed by most developers):

=============================== ============================= ================
Type Directory Mandatory?
=============================== ============================= ================
Commands ``Command/`` Yes
Controllers ``Controller/`` No
Service Container Extensions ``DependencyInjection/`` Yes
Event Listeners ``EventListener/`` No
Model classes [1] ``Model/`` No
Configuration ``Resources/config/`` No
Web Resources (CSS, JS, images) ``Resources/public/`` Yes
Translation files ``Resources/translations/`` Yes
Templates ``Resources/views/`` Yes
Unit and Functional Tests ``Tests/`` No
=============================== ============================= ================

[1] See :doc:`/doctrine/mapping_model_classes` for how to handle the
mapping with a compiler pass.
=============================== ======================================== ==========
Type Directory Mandatory?
=============================== ======================================== ==========
Commands ``Command/`` Yes
Controllers ``Controller/`` No
Service Container Extensions ``DependencyInjection/`` Yes
Doctrine entities and documents ``Entity/`` (ORM) or ``Document/`` (ODM) No
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because thanks to the doctrine.orm.mapings you can store your entities anywhere you like, even outside the bundles.

In any case, I dislike this Mandatory 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.

Copy link
Member

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.

Copy link
Member

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

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for removing this column.

Copy link
Member

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 the 3.4 branch because auto-discovering commands based on their location will be deprecated

Event Listeners ``EventListener/`` No
Configuration ``Resources/config/`` No
Web Resources (CSS, JS, images) ``Resources/public/`` Yes
Translation files ``Resources/translations/`` Yes
Copy link
Contributor

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?

Copy link
Member Author

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.

Templates ``Resources/views/`` Yes
Unit and Functional Tests ``Tests/`` No
=============================== ======================================== ==========

Classes
-------
Expand Down