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

IBX-374: Fixed retrieving factory names in LazyDoctrineRepositoriesPass #3103

Merged
merged 2 commits into from
May 25, 2021

Conversation

webhdx
Copy link
Contributor

@webhdx webhdx commented May 13, 2021

Question Answer
JIRA issue IBX-374
Bug yes
New feature no
Target version 7.5
BC breaks no
Tests pass yes
Doc needed no

###Note to QA:

You can't really test this from user perspective. I recommend doing sanities around installation process and container building (via cache:clear).

I remember issue mentioned by Edi to be reproducible on 3.2 or 3.3 as this fix was already implemented there. We can try reverting it for 3.2 and 3.3 so we can find out what was exactly causing this issue to replicate the same setup on 2.5.

If you want to test it really deeply I think you can go over whole Entity Manager testing procedure.

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

LazyDoctrineRepositoriesPassTest looks like a good place to add coverage for this case, the solution looks ok.

@webhdx
Copy link
Contributor Author

webhdx commented May 17, 2021

@alongosz Added coverage for this case in existing test: 0f97074

@webhdx webhdx requested a review from alongosz May 17, 2021 11:34
@bogusez bogusez self-assigned this May 21, 2021
@webhdx webhdx merged commit 403b2cc into 7.5 May 25, 2021
@webhdx webhdx deleted the lazy-doctrine-factory-pass-fix branch May 25, 2021 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants