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
Allows both doctrine/common 3.x & doctrine/persistence 2.x #349
Conversation
I think I already expressed them in another of your PRs, but IMO you should start contributing a maximum of things to the stable branch first. This applies to allowing common 3 and persistence 2 I think. |
what about allowing doctrine/mongodb-odm 2.1.x? Required to allow doctrine/persistence 2.x (see doctrine/mongodb-odm#2204 Ok to drop doctrine/mongodb-odm 1.x too? |
If both versions can be handled by a single version of data-fixtures, why not?
Preferrably in a subsequent PR. |
Related PR in mongodb-odm has been merged |
tests/Doctrine/Tests/Common/DataFixtures/Purger/MongoDBPurgerTest.php
Outdated
Show resolved
Hide resolved
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- Allow doctrine/persistence 2 | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A Replaces #37416. This PR unblocks the installation of `doctrine/persistence` version 2. Notes: * I had to bump `doctrine/data-fixtures` because composer would otherwise downgrade to an ancient alpha release with incomplete version constraints. This package is a dev dependency for our tests, so I think we're good here. **edit**: Moved to #37640. * Until doctrine/data-fixtures#349 has been resolved, Doctrine/Bridge cannot be tested with version 2 of `doctrine/persistence`. Commits ------- cd22fe6 Allow doctrine/persistence 2
tests seem to fail on registering annotations. The previously used |
The error message says it |
Targets the 1.4.x stable branch now. Will bump doctrine/mongodb-odm in another pull request |
"doctrine/common": "^2.13|^3.0", | ||
"doctrine/persistence": "^1.3.3|^2.0" |
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.
It looks like none of the builds is installing versions fitting to the version constraints ^3.0
and ^2.0
. How can we be sure that the newer versions will work too?
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.
How can they be newer? 3 and 2 are the latest versions…
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.
Both doctrine/common 3 and doctrine/persistence 2 will not install on dev, because of doctrine/mongodb-odm 1.3.x (latest version is 2.1.x) not having support for doctrine/common 3.
A few options:
- Add support for doctrine/mongodb-odm 2.x & drop support for doctrine/mongodb-odm 1.3.x in this pull request. They don't go together nicely (tried to in a previous version of this pull request)
- After this pull request is merged upstream, go for the first option but on the dev branch instead.
- Replace doctrine/common in doctrine/mongodb-odm 1.3.x with the new doctrine packages like done in 2.x](doctrine/mongodb-odm@21501c4) could try something like this?
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.
Option 3 sounds best, can you manage that?
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.
Worth a try ;)
Quite the task tho. A little help would be nice. Several classes (1) are no longer available due to the removal of doctrine/common. They will have to be replaced in a way, for which I've found docs https://www.doctrine-project.org/2018/07/12/common-2-9-and-dbal-2-8-and-orm-2-6-2.html
For example, to get proxying to work, we'll have to backport changes like doctrine/mongodb-odm#1875 This results in BC breaks, which I don't think is acceptable on this stable branch. Am I missing something?
(1) Classes no longer available;
Doctrine\Common\Proxy\AbstractProxyFactory
Doctrine\Common\Proxy\ProxyDefinition
Doctrine\Common\Proxy\ProxyGenerator
Doctrine\Common\Util\ClassUtils
Doctrine\Common\Util\Debug
Doctrine\Common\Proxy\Proxy
Will give supporting both doctrine/mongodb-odm 1.x & 2.x another try too at the end of the week.
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.
The classes above are still available in common v3. Why not upgrade to common 3 + replacement packages in v1 of doctrine/mongodb-odm
?
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.
@fruitwasp after taking a deeper look to this, I think option 1 might be best in fact… Apparently doctrine/mongodb-odm
is just a dev dependency, which means people can already install v2 alongside doctrine fixtures, right?
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.
With data-fixtures as a dependency in a project, its require-dev parts won't be affected by a v2 constraint. Suggestion 2 would be possible.
How will this affect the build? Will there be one build that uses e.g. Common ^2.13 and a different one ^3.0? The compatibility with the code of all the given constraints of doctrine/common
and doctrine/persistence
should be tested in builds.
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.
@SenseException now that #351 is merged, we have the prefer lowest build that uses 1 and 2, while all other builds use 2 and 3.
Hi! Giving this a gentle bump :). This library (and DoctrineFixturesBundle) is no longer installable out-of-the-box on Symfony 5 apps (ref):
It looks like there is a MongoDB issue that we're still checking into? Is that all? Thanks! |
I think this package needs to be made compatible with |
#351 has been merged, let's trigger a new build |
Looks like doctrine/common 3 & /persistence have been installed in the builds. Great work! |
Thanks @fruitwasp ! |
Summary
Thoughts?