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

Don't rely on LazyProxyTrait in LazyServiceEntityRepository #1727

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Don't rely on LazyProxyTrait in LazyServiceEntityRepository #1727

merged 1 commit into from
Nov 14, 2023

Conversation

nicolas-grekas
Copy link
Member

Makes things simpler and unlocks symfony/symfony#52568

@nicolas-grekas nicolas-grekas mentioned this pull request Nov 13, 2023
Merged
@nicolas-grekas
Copy link
Member Author

Could anyone help me with the psalm failure?

@nicolas-grekas nicolas-grekas force-pushed the ve-less-lazy-repo branch 2 times, most recently from 083c0c8 to e86cc6d Compare November 13, 2023 18:39
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Nov 13, 2023 •
edited
Loading

Error: Property \Doctrine\Bundle\DoctrineBundle\Repository\LazyServiceEntityRepository::$registry does not have native type hint for its value but it should be possible to add it based on @var annotation "ManagerRegistry".

Wrong hint by phpcs since we should be compatible with PHP 7.4, isn't it?

nvm, typed props are allowed in 7.4 🤦

@derrabus
Copy link
Member

Could anyone help me with the psalm failure?

I have no idea. This ancient Psalm version on this repository has become a burden, tbh.

@stof stof added this to the 2.11.1 milestone Nov 14, 2023
@stof
Copy link
Member

stof commented Nov 14, 2023

@derrabus I suggest that we update the Psalm setup to use Psalm 5 instead. It is probably easier than debugging Psalm 4...

@ostrolucky
Copy link
Member

Yeah we should update it. I didn't even realize it's so outdated TBH. Psalm 5 supports PHP 7.4, so that shouldn't be an issue either.

@derrabus
Copy link
Member

@derrabus I suggest that we update the Psalm setup to use Psalm 5 instead.

I've tried a couple of days ago. If you upgrade Psalm, it spits out seventy-something errors that should be reviewed. Whoever wants to give it a try: be my guest. 🙂

@derrabus
Copy link
Member

Okay then, screw Psalm for now. This should not block us. Thank you @nicolas-grekas.

@derrabus derrabus merged commit 8c42712 into doctrine:2.11.x Nov 14, 2023
11 of 12 checks passed
@ostrolucky
Copy link
Member

ostrolucky commented Nov 14, 2023 •
edited
Loading

I'll work on updating to psalm 5 soon

@nicolas-grekas nicolas-grekas mentioned this pull request Nov 15, 2023
Merged
@nicolas-grekas nicolas-grekas deleted the ve-less-lazy-repo branch November 15, 2023 18:14
@github-actions github-actions bot mentioned this pull request Nov 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus left review comments

@ostrolucky ostrolucky ostrolucky approved these changes

Assignees
No one assigned
Labels
Improvement
Projects
None yet
Milestone
2.11.1
Development

Successfully merging this pull request may close these issues.
4 participants
@nicolas-grekas @derrabus @stof @ostrolucky