Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Symfony 5 #324

Merged
merged 7 commits into from
Dec 12, 2019
Merged

Symfony 5 #324

merged 7 commits into from
Dec 12, 2019

Conversation

gilles-g
Copy link
Member

@gilles-g gilles-g commented Dec 10, 2019

@garak

I created a new PR and made some little changes in travis.yml and TestCase.
I remove tests for SF 2.8 because we need to require doctrine/doctrine-bundle but this bundle required SF 3.4...
And I looked at the other bundles a bit, and kept the master branch compatible with 2.8 is no longer really a relevant thing.

But composer failed with SF 5.0 and doctrine/mongodb-odm-bundle if you have an idea?

  • sf 3.4 removing
  • sf 4.3
  • sf 4.4
  • sf 5.0

composer.json Outdated
},
"require-dev": {
"doctrine/mongodb-odm": "^1.1|^2.0",
"doctrine/mongodb-odm-bundle": "^3.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to allow mongodb-odm-bundle 4 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 tried, but mongo-odm-bundle 4 removed https://github.com/doctrine/DoctrineMongoDBBundle/blob/3.5/DataCollector/PrettyDataCollector.php in 4.*

And brokes our tests, maybe I could change the tests 🙄

Copy link
Member Author

Choose a reason for hiding this comment

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

But in 3.* we have PrettyDataCollector.php and StandardDataCollector.php
In 4.* we have only CommandDataCollector.php

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I can't find anything in mongo-odm-bundle mentioting the removal of PrettyDataCollector. Anyway, can't you just use CommandDataCollector instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I opened an issue to see if we can get some feedback from bundle maintainers: doctrine/DoctrineMongoDBBundle#606

Copy link
Member Author

@gilles-g gilles-g Dec 10, 2019

Choose a reason for hiding this comment

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

I tried to use CommandDataCollector, but ODM bundle brokes a lot of codes :/

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a hard BC break regarding data collectors from 3.x to 4.x. Unfortunately, we can't really fix that as 3.x does not know about the commands. This is a side effect of the driver change underneath the hood.

As for the original issue, there's no point adding ^2.0 for ODM if you don't allow 4.0 for the bundle: 3.x is not compatible with ODM 2.0 and will prevent installing it anyways.

Last but not least, you should also change the constraint for ODM to ^1.3|^2.0, as 1.1 has been EOL'd a while ago and doesn't contain the forward compatibility and deprecation layer for 2.0.

@gilles-g
Copy link
Member Author

@garak
I have found this PR doctrine/DoctrineMongoDBBundle#604

This PR fixes this issue by removing support for Symfony 3.4, which will be released as 4.1.0

🤔

Well maybe we need to take the decision to create a branch support for SF 3.4 and an other one for SF 4/5..

@alcaeus Did you think is the right thing to do?

@garak
Copy link
Contributor

garak commented Dec 12, 2019

I absolutely agree on that.
Supporting more than 2 major versions in the same codebase is just a pain.

@alcaeus
Copy link
Contributor

alcaeus commented Dec 12, 2019

Yes, it's not possible to support Symfony 3.4 and 5.0 if you have a data collector. I haven't gotten around to finishing this in the ODM bundle, but will try to do so early next week.

@alcaeus
Copy link
Contributor

alcaeus commented Dec 12, 2019

One more word: I would highly suggest splitting off the ODM 2.0 change from this PR: 1.3 and the corresponding 3.6 version of the bundle support Symfony 5; there's no need to worry about ODM 2.0 right now. You should use a separate PR for that as it may entail significantly more changes.

@gilles-g
Copy link
Member Author

Ok thanks for your answer! 👍

@garak If you have an advice for branch..

Could we have futur tags v5.* on a new branch named 3.4?

And the a futur tags v6.* on master? Or a branch 4.4 is needed?

@garak
Copy link
Contributor

garak commented Dec 12, 2019

@Spike31 tag names don't depend on branch names, but I'd keep consistency just for a matter of human readability

@gilles-g
Copy link
Member Author

Yes it's just for context:

SF 2.8/3.4 => branch 3.4 => tags until v5.*
SF 4.* => branch master => tags v6.*

@gilles-g
Copy link
Member Author

@garak
What did you think about the PR now?

I have to remove DataCollector because in the new ODM bundle there no more Logger to get Collector...Methods was deprecated and deleted!

Anymay, I think we could merge into master and create a new major version

@garak
Copy link
Contributor

garak commented Dec 12, 2019

LGTM 👍

@gilles-g
Copy link
Member Author

Congratulations on your arrival in the Symfony organization!

@gilles-g gilles-g merged commit 4f88c5a into lexik:master Dec 12, 2019
@gilles-g
Copy link
Member Author

close #322

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

Successfully merging this pull request may close these issues.

3 participants