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

[RFC] Do not force to use annotations for Sylius project #335

Open
wants to merge 2 commits into
base: 1.12
Choose a base branch
from

Conversation

Zales0123
Copy link
Member

I open this PR as a base for a discussion about default orm mapping in Sylius-Standard.

Right now we have all Sylius entities extended by default in src/Entity/ directory and mapped with annotations mapping - which is correct and useful, as we have no better option to extend Sylius entities in plugins than providing a trait with new properties, including their mapping. So with such a configuration installing a plugin and extending a Sylius entity is quite hassle-free.

HOWEVER

I, personally, feel bad that I'm a little bit forced to use annotations (I really prefer yml or, even better, xml mapping). I know, I'm not the only one, even though there are not so many of us nowadays 🏝 👴 When I create my own, new resource, in src/Entity/ directory, I'm not able to change the mapping type that I hate, without changing it for all other entities. I was struggling with this issue, as I don't want to force anyone to follow my rules, but on the other hand, I don't want to be forced to anything neither 🚀 Moreover, a doctrine mapping configuration is not so smart and we cannot define any rule like "use this mapping or, if it's not defined, use this instead" 😢 The only way to differentiate mappings is to separate entities into subdirectories and configure mapping for them.

As a first proposal, I wanted to move all Sylius entities to src/Entity/Sylius and create another src/Entity/Main directory with different configurations. I found it kinda weird, so I propose a separate configuration for each pack of entities and another one for a src/Entity/Main (name to be refactored), as a directory for custom entities.

I don't know is it the best solution. More, I don't even know if maybe it's my imagined problem 😄 But I would love to see an opinion of others @Sylius/core-team members and, of course, from our beloved community 🎉

Live long and prosper 🖖 ☮️

@Zales0123 Zales0123 added the RFC label Mar 19, 2019
@Zales0123 Zales0123 requested a review from a team as a code owner March 19, 2019 09:53
@Zales0123 Zales0123 force-pushed the better-mapping-configuration branch from 0b0231f to c1dd292 Compare March 19, 2019 09:55
@Zales0123 Zales0123 force-pushed the better-mapping-configuration branch from c1dd292 to 099937e Compare July 3, 2019 08:11
@Zales0123 Zales0123 force-pushed the better-mapping-configuration branch from 099937e to 49f6321 Compare July 3, 2019 08:13
@@ -19,6 +19,90 @@ doctrine:
App:
is_bundle: false
type: annotation
dir: '%kernel.project_dir%/src/Entity'
prefix: 'App\Entity'
dir: '%kernel.project_dir%/src/Entity/Main'
Copy link
Contributor

Choose a reason for hiding this comment

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

Better names: App, Custom, Application

@loevgaard
Copy link
Contributor

Regarding the actual problem I always use XML in bundles/plugins (which I make a lot of :D) so I am also a big fan of XML, but I am also a big fan of annotations within applications. It's very easy to read, and if I want to use third party plugins they will usually provide an annotated trait, which also makes it very easy.

But I don't see anything wrong with this PR. It gives the flexibility you are looking for at least :)

@Ferror
Copy link
Contributor

Ferror commented May 24, 2022

@Zales0123 Could you decide to kill or make this feature work? It's been 3 years and I don't see much activity here 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants