-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Remove last references to ClassMetadataInfo #9713
Conversation
05c897c
to
ef84d64
Compare
* @param string $entityName The name of the entity class the new instance is used for. | ||
* @psalm-param class-string<T> $entityName | ||
* @var string | ||
* @todo Not really needed. Usage could be localized. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what "Usage could be localized" means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 me neither
This change will be a PITA for future merge-ups. 😓 |
@derrabus you mean when touching CMI? Git detected it as a rename, so it should be transparent. |
Are you sure? Unless GitHub has a very strage way to display a file rename, it looks like a big code chunk has been moved from one class to another. |
It's a bit confusing:
Let me try doing a local merge up. |
When trying a local merge up, there is indeed a conflict, where none of the choices are good (I get a choice between deleted file and modified file, but CM is not modified when I pick "deleted file") |
I tried removing |
28012e9
to
f420d0c
Compare
It is unused apart from a tests where it is easily replaced.
It has been deprecated for a long, long time.
@derrabus I've postponed the removal of CMI itself, the rest can go now IMO. |
It has been deprecated for a long, long time.
ClassMetadataInfo
itself is left to avoid conflict. It shall be removed right before releasing 3.0.0