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.

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

Conversation

javiereguiluz
Copy link
Member

No description provided.

@wouterj
Copy link
Member

wouterj commented Nov 22, 2016

👍
status: reviewed

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.

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

@weaverryan weaverryan merged commit 8764708 into 2.7 Nov 2, 2017
weaverryan added a commit that referenced this pull request Nov 2, 2017
…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 classes

Commits
-------

8764708 Final updates
469d0ea Added Doctrine entities and documents to the list of known locations for classes
@xabbuh xabbuh deleted the bundle_classes branch November 2, 2017 13:49
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.

7 participants