-
Notifications
You must be signed in to change notification settings - Fork 453
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
Improve setup docs regarding Doctrine ODM #483
Improve setup docs regarding Doctrine ODM #483
Conversation
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.
👍
Some prefer XML, other prefer YML. You seem to prefer annotations. IMO would be best to have as much docs and examples as possible. P.S Merging |
@dinamic See PR description:
As for Doctrine recommendations, we recommend using XML for libraries or when you want to decouple your domain logic from your mapping. In other cases, use annotations. |
There shouldn't be "other cases", when do you NOT want to do this? |
One could argue that even using XML ties your domain logic to your mapping, but that's outside the scope here. I was merely stating the reason for the change. |
869f05a
to
5989d0d
Compare
Well, you obviously need the mapping somewhere, but IMO promoting annotations has done some harm here, it's now seen as a go-to way of doing things even though it shouldn't be. |
In that case, feel free to change the ORM documentation. The primary goal was to have consistent documentation and ensure the ODM documentation produces a working result. |
Sorry, didn't mean (just) here, I mean overall in the community. Annotations should be a quick way to do something short-term, but in the long run you'll end up wanting to have used something else. :) |
That's subjective - while not optimal I believe annotations provide a nice balance between speed and "clean code feel". Returning to the topic, what would you suggest for the issue at hand? ODM docs are actually broken and should be fixed. Would you rather drop XML for now and reintroduce it in a docs expansion later or would you rather change ORM docs first? |
Exactly why we should follow the established way of having all three in the docs. Is there any way we could do that? |
Please let's leave out YAML. We're dropping support for YAML in ODM 2.0 and ORM 3.0. |
That's 1/3 of work already done! 😆 So we need XML for ORM and annotations for ODM? Seems doable, do you agree? |
Absolutely. @winterstefan can you make the changes for ODM? I can make the changes for ORM unless you prefer to do them as well 😉 |
@winterstefan still up to work on this PR or would you like someone else to finish up? Note that #558 brings in annotations for ODM, so we could take that part of the docs from there. |
@alcaeus Sorry, my bad. Actually I'm pretty out of this topic. Would be cool for someone else to finish up. |
Fair enough, I know just the guy to do it 😉 |
Goal:
Steps:
user
field where necessary (ORM already has them).Doctrine\ODM\MongoDB\Mapping\Annotations
configuration (same as for ORM).getClient()
andsetClient()
since the super class already implements getter / setter.