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

Jump to bottom

Preload Autoloader class when the ORM is used #1167

Merged
merged 1 commit into from May 25, 2020
Merged

Preload Autoloader class when the ORM is used #1167

merged 1 commit into from May 25, 2020

Conversation

fancyweb
Copy link
Contributor

The Autoloader class is always used on bundle boot when the ORM is used.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 15, 2020 •
edited

What about using a class_exists() at the top of DoctrineBundle.php?
Should we list ClassUtils also?

@fancyweb
Copy link
Contributor Author

What about using a class_exists() at the top of the DoctrineBundle file?

That's what I did first until I realized there is a condition on boot: $this->container->hasParameter('doctrine.orm.proxy_namespace')

The class is hit only when the ORM is used.

@alcaeus
Copy link
Member

alcaeus commented May 19, 2020

Does this fix a bug in 2.0.x? Otherwise I'd opt to introduce this in 2.1.0 which will be released this week.

@fancyweb
Copy link
Contributor Author

Technically it does not a fix a bug, it's an improvement. But I need to take care of tests.

@alcaeus alcaeus changed the base branch from 2.0.x to master May 19, 2020 13:43
@alcaeus alcaeus changed the base branch from master to 2.0.x May 19, 2020 13:43
@alcaeus
Copy link
Member

alcaeus commented May 19, 2020

In that case, let's push this into master. I didn't change the base (yet) as master seems to be missing an up merge from 2.0.x. Either way, let's try to get this into 2.1. Let me know if you need help

@alcaeus alcaeus changed the base branch from 2.0.x to master May 25, 2020 12:41
@alcaeus alcaeus added this to the 2.1.0 milestone May 25, 2020
alcaeus
alcaeus reviewed May 25, 2020
View reviewed changes
Tests/DependencyInjection/Fixtures/config/yml/orm_service_single_entity_manager.yml
@@ -10,7 +10,7 @@ doctrine:
memory: true

orm:
default_entity_manager: dm2
default_entity_manager: default
Copy link
Member

Choose a reason for hiding this comment

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

The new logic above uncovered this error in the configuration. Looks like we don't check whether the default entity manager even exists 🤷‍♂️

@alcaeus alcaeus self-assigned this May 25, 2020
@alcaeus alcaeus merged commit 903c248 into doctrine:master May 25, 2020
@alcaeus
Copy link
Member

alcaeus commented May 25, 2020

Thanks @fancyweb!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alcaeus alcaeus alcaeus approved these changes

Assignees

@alcaeus alcaeus

Labels
Improvement
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@fancyweb @nicolas-grekas @alcaeus @ostrolucky