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.

Sign up for GitHub

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

Jump to bottom

[make:*] improve support for ORM 3.x / DBAL 4.x #1439

Merged
merged 24 commits into from
Feb 6, 2024
Merged

[make:*] improve support for ORM 3.x / DBAL 4.x #1439

merged 24 commits into from
Feb 6, 2024

Conversation

jrushlow
Copy link
Collaborator

@jrushlow jrushlow commented Feb 5, 2024 •
edited
Loading

  • bumps the required doctrine/orm version to ^2.15|^3
  • bumps minimum doctrine/doctrine-bundle to ^2.10
  • handles new Mapping classes for entites. (FieldMapping, ManyToOneMapping, etc...)
  • adds a MAKER_PROCESS_TIMEOUT env that can be set to null or a float. This overrides the default Process::timeout value of 60 for shell commands in test. (Allows step debugging w/ XDebug)
  • tmp disable Mercure tests on CI. ux/turbo uses Doctrine\Common\Util\ClassUtils which no longer exists. (Fails all of our tests) See comment in review.
  • adds legacy tests for ORM 2.x entity attributes that "reversed" property order compared to ORM 3.x
  • ORM\..\DisconnectedClassMetadataFactory is removed in orm 3.0 -> set the StaticReflectionService::class manually when getting metadata for disconnected entities. (e.g. xml mapping) - Reference ClassMetadataInfo less and less doctrine/orm#9704

@jrushlow jrushlow changed the title [ci] don't use database:create for sqlite in tests [ci] new doctrine compatibility / don't use database:create for sqlite in tests Feb 5, 2024
@jrushlow jrushlow changed the title [ci] new doctrine compatibility / don't use database:create for sqlite in tests new doctrine compatibility / don't use database:create for sqlite in tests Feb 5, 2024
jrushlow
jrushlow commented Feb 6, 2024
View reviewed changes
phpunit.xml.dist
Comment on lines +22 to +23
<!-- Overrides process timeout when step debugging -->
<!-- <env name="MAKER_PROCESS_TIMEOUT" value="null" /> -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Allows for debugging Process::fromShellCommandLine() which sets the process $timeout to 60 seconds

jrushlow
jrushlow commented Feb 6, 2024
View reviewed changes
src/Util/ClassSource/Model/ClassProperty.php
*
* @internal
*/
final class ClassProperty
Copy link
Collaborator Author

@jrushlow jrushlow Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We'll be able to do additional refactorings in CSM/Maker that will further utilize value object.. e.g. addProperty()

jrushlow
jrushlow commented Feb 6, 2024
View reviewed changes
.github/workflows/ci-linux.yaml
@@ -11,6 +11,7 @@ on:
env:
PHPUNIT_FLAGS: "-v"
SYMFONY_PHPUNIT_DIR: "$HOME/symfony-bridge/.phpunit"
MAKER_SKIP_MERCURE_TEST: 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ux-turbo uses Doctrine\Commom\Util\ClassUtils::class which no longer exists. As such, 2 MakeEntity tests fail that use Mercure. Keep this here && create an maker bundle issue to fix? Or live with (for now) the red x's on all of our tests?

Copy link
Member

Choose a reason for hiding this comment

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

Keep this for now. Then we'll fix it in ux-turbo at some point.

@jrushlow jrushlow changed the title new doctrine compatibility / don't use database:create for sqlite in tests [make:*] improve support for ORM 3.x / DBAL 4.x Feb 6, 2024
@jrushlow jrushlow added Feature New Feature Status: Needs Review Needs to be reviewed labels Feb 6, 2024
@jrushlow jrushlow marked this pull request as ready for review February 6, 2024 17:50
weaverryan
weaverryan approved these changes Feb 6, 2024
View reviewed changes
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

This looks like very detailed work & solid 🦾. If any details were missed, the tests should have caught them.

.github/workflows/ci-linux.yaml
@@ -11,6 +11,7 @@ on:
env:
PHPUNIT_FLAGS: "-v"
SYMFONY_PHPUNIT_DIR: "$HOME/symfony-bridge/.phpunit"
MAKER_SKIP_MERCURE_TEST: 1
Copy link
Member

Choose a reason for hiding this comment

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

Keep this for now. Then we'll fix it in ux-turbo at some point.

@jrushlow jrushlow added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Feb 6, 2024
@jrushlow jrushlow merged commit d3a0cdb into symfony:main Feb 6, 2024
5 of 6 checks passed
@jrushlow jrushlow deleted the ci/handle-database-create branch February 6, 2024 20:12
This was referenced Feb 6, 2024
Merged
Merged
@kbond kbond mentioned this pull request Feb 7, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weaverryan weaverryan weaverryan approved these changes

Assignees
No one assigned
Labels
Feature New Feature Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jrushlow @weaverryan