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

abstain from registering incomplete services #950

Merged
merged 1 commit into from Apr 8, 2019

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Apr 7, 2019

fixes symfony/symfony#30948

#FOSSHackathons #EUFOSSA

@xabbuh
Copy link
Member Author

xabbuh commented Apr 7, 2019

ping @dmaicher @dunglas

@dmaicher
Copy link
Contributor

dmaicher commented Apr 7, 2019

Can confirm that this fixes the issue.

But it also means (without having symfony/property-info) I cannot enable the new doctrine validator mapping anymore so it actually works? Or am I missing something?

Copy link
Contributor

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

We’ll have to add property-info in the pen-pack.

@xabbuh
Copy link
Member Author

xabbuh commented Apr 7, 2019

something else seems to be wrong, @dmaicher and me are investigating

@xabbuh xabbuh changed the base branch from 1.10.x to master April 7, 2019 10:44
@xabbuh
Copy link
Member Author

xabbuh commented Apr 7, 2019

In fact, this was not an issue on 1.10.x at all. I picked the code that already prevent this from happen into master.

This is ready to be reviewed.

@dmaicher
Copy link
Contributor

dmaicher commented Apr 7, 2019

@alcaeus somehow this

https://github.com/doctrine/DoctrineBundle/blob/1.10.x/DependencyInjection/DoctrineExtension.php#L358

was lost during some merge up into master?

Copy link
Contributor

@dmaicher dmaicher left a comment

Choose a reason for hiding this comment

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

👍

@alcaeus alcaeus added Bug ⭐️ EUFOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming labels Apr 7, 2019
@alcaeus alcaeus self-assigned this Apr 7, 2019
@alcaeus
Copy link
Member

alcaeus commented Apr 7, 2019

This was inadvertently removed in #892. Thanks @xabbuh and @dmaicher for digging into this. I'll see what I can do to prevent this in the future.

@alcaeus alcaeus modified the milestones: 1.10.3, 1.11.0 Apr 7, 2019
@alcaeus alcaeus added this to 1.11 in Roadmap Apr 7, 2019
@alcaeus alcaeus merged commit 5ad6645 into doctrine:master Apr 8, 2019
@alcaeus
Copy link
Member

alcaeus commented Apr 8, 2019

Thanks @xabbuh!

@xabbuh xabbuh deleted the symfony-30948 branch April 8, 2019 07:18
@xabbuh
Copy link
Member Author

xabbuh commented Apr 8, 2019

Thank you for fixing the class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug ⭐️ EUFOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DoctrineBridge] dependency on symfony/property-info missing?
5 participants