Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Symfony 7 support #784

Merged
merged 4 commits into from
Dec 13, 2023
Merged

Symfony 7 support #784

merged 4 commits into from
Dec 13, 2023

Conversation

kbond
Copy link
Contributor

@kbond kbond commented Oct 25, 2023

@kbond
Copy link
Contributor Author

kbond commented Oct 25, 2023

Also fixed the workflow - it wasn't properly setting SYMFONY_REQUIRE

Oi, this opened a can of worms as the bundle hasn't been properly tested by the CI since #678.

@kbond kbond force-pushed the sf7 branch 2 times, most recently from fd30cbd to e5f3caf Compare October 25, 2023 14:02
@franmomu
Copy link
Contributor

Looks like it's installing symfony/string 5.4.0-RC1 in https://github.com/doctrine/DoctrineMongoDBBundle/actions/runs/6641537314/job/18044298016?pr=784#step:7:207 instead of a stable one

@malarzm
Copy link
Member

malarzm commented Oct 25, 2023

Just merged #785, shall we release patch version, merge up, and rebase this PR?

@chalasr
Copy link
Contributor

chalasr commented Nov 27, 2023

I need this to make api-platform Symfony7-ready
Do you need help fixing the remaining tests @kbond?

@kbond
Copy link
Contributor Author

kbond commented Nov 27, 2023

@chalasr, yes, if you don't mind :)

@kbond kbond force-pushed the sf7 branch 2 times, most recently from 31dcab2 to c969393 Compare December 9, 2023 10:04
@GromNaN GromNaN changed the title feat: Symfony 7 support feat: Symfony 7 support + remove deprecation for new v5 Dec 9, 2023
@GromNaN GromNaN changed the base branch from 4.7.x to 5.0.x December 9, 2023 11:02
@GromNaN GromNaN requested review from alcaeus and malarzm December 9, 2023 11:22
Comment on lines 216 to 221
*
* @return void
*/
protected function loadDocumentManager(array $documentManager, $defaultDM, $defaultDB, ContainerBuilder $container)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
*
* @return void
*/
protected function loadDocumentManager(array $documentManager, $defaultDM, $defaultDB, ContainerBuilder $container)
*/
protected function loadDocumentManager(array $documentManager, $defaultDM, $defaultDB, ContainerBuilder $container): void

It would be nice to revisit later if protected makes sense.

@GromNaN GromNaN changed the title feat: Symfony 7 support + remove deprecation for new v5 feat: Symfony 7 support Dec 10, 2023
@GromNaN GromNaN added this to the 5.0.0 milestone Dec 10, 2023
@GromNaN GromNaN added the Task label Dec 10, 2023
@GromNaN GromNaN self-assigned this Dec 10, 2023
@GromNaN GromNaN force-pushed the sf7 branch 4 times, most recently from 70395bc to 9777b54 Compare December 11, 2023 09:10
@GromNaN GromNaN requested review from franmomu and GromNaN December 11, 2023 09:59
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@franmomu franmomu left a comment

Choose a reason for hiding this comment

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

You can also remove this

// available in Symfony 6.2 and higher
if (! class_exists(EntityValueResolver::class)) {
return;
}

"symfony/yaml": "^5.4 || ^6.2",
"symfony/browser-kit": "^6.4 || ^7.0",
"symfony/form": "^6.4 || ^7.0",
"symfony/phpunit-bridge": "^6.4.1 || ^7.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"symfony/phpunit-bridge": "^6.4.1 || ^7.0.1",
"symfony/phpunit-bridge": "^7.0.1",

I guess we can use only the latest

Copy link
Member

@GromNaN GromNaN Dec 12, 2023

Choose a reason for hiding this comment

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

Yes, both are almost identical. I don't want to conflict with the SYMFONY_REQUIRE=6.4.*

Copy link
Contributor

@franmomu franmomu left a comment

Choose a reason for hiding this comment

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

the CS job is failing

@GromNaN GromNaN merged commit f190d18 into doctrine:5.0.x Dec 13, 2023
@GromNaN
Copy link
Member

GromNaN commented Dec 13, 2023

Thank you @kbond for starting this work. As you said, it wasn't as simple as it looked.

@GromNaN GromNaN changed the title feat: Symfony 7 support Symfony 7 support Dec 23, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants