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

[WIP] Add Doctrine ORM support #148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Add Doctrine ORM support #148

wants to merge 1 commit into from

Conversation

tgalopin
Copy link

This pull request add the support of ORM in the SimpleCmsBundle.

It is a work in progress for the moment.

@tgalopin tgalopin changed the title [WIP] Add ORM model class, entity, functionnal and web tests [WIP] Add ORM support Aug 11, 2015
@tgalopin tgalopin changed the title [WIP] Add ORM support [WIP] Add Doctrine ORM support Aug 11, 2015
->end()
->end()
->end()
*/
Copy link
Member

Choose a reason for hiding this comment

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

we need to either implement sonata or remove these commented out configuration parts

@dbu
Copy link
Member

dbu commented Aug 11, 2015

thanks a lot! i added some note about the things we said we still need to work on.

@@ -14,6 +14,7 @@
"minimum-stability": "dev",
"require": {
"php": ">=5.3.9",
"doctrine/orm": "~2.2,>=2.2.3,<2.5",
Copy link

Choose a reason for hiding this comment

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

Why not doctrine/orm 2.5? Tip: ~2.2,>2.2.3 can be replaced with ^2.2.3 https://getcomposer.org/doc/articles/versions.md#caret.

Copy link
Member

Choose a reason for hiding this comment

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

this was taken straight from symfony standard edition 2.7:
https://github.com/symfony/symfony-standard/blob/2.7/composer.json#L12

but indeed, we should simply say ^2.2.3 and leave it to the
application to know if they have an issue with the orm or not.

Copy link
Member

Choose a reason for hiding this comment

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

2.5 has a BC break with 2.4 with the setup used by the Symfony standard (you had to configure some new setting, otherwise the DB connection couldn't be made, breaking all Symfony pages, as connections were always made).

For a package, 2.5 can be completely valid. When changing to ^2.2.3, please also update the rest of the dependencies to all use the same operators (for consistency, I'm slowly upgrading all CMF packages to use ^ instead of ~).

@dbu dbu modified the milestone: 1.3 Aug 21, 2015
@tgalopin
Copy link
Author

It's been a while but I'm still planning to do this. I'm pretty busy right now with personnal stuff, but I'll work on this soon :) .

@wouterj
Copy link
Member

wouterj commented Aug 30, 2015

I think we should remove the 1.3 milestone here. It's a very great feature (and one that I want to have in the CMF since 1.0), but 1.3 is coming too early to have this included. Furthermore, if we skip this for 2.0 (end of this year), we can add ORM support to more bundles and release them all at once.

@wouterj wouterj removed this from the 1.3 milestone Aug 30, 2015
@dbu
Copy link
Member

dbu commented Aug 30, 2015

agreed its no blocker for 1.3, but if we manage to get it merged in time, i would not hold back in the hope of more orm support - we can also add it step by step

@dbu
Copy link
Member

dbu commented Oct 9, 2015

ping @tgalopin

@tgalopin
Copy link
Author

I'm currently really drowning into work, so even if I planned to work on this, it's a way too big PR for me right now. If someone is interested in continuing the work, please do!

Sorry :/ !

@tgalopin tgalopin closed this Jan 23, 2016
@lsmith77 lsmith77 removed the wip/poc label Jan 23, 2016
@dbu
Copy link
Member

dbu commented Jan 25, 2016

i think the code is still relevant. reopening - when somebody has time to wrap this up, this will be a good starting point.

@dbu dbu reopened this Jan 25, 2016
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.

5 participants