-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[DoctrineBridge] prevent deprecated message #54255
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
Conversation
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Hey! Thanks for your PR. You are targeting branch "6.4" but it seems your PR description refers to branch "6.4,". Cheers! Carsonbot |
I rebased the PR for 5.4 where the issue exists. It's strange that we didn't notice the deprecation before. It likely means that this code path is not tested. Could you please have a look and add a test case if none covers this line? |
Problem with V5.4 is compatible with Doctrine/Orm < 2.10.3 where change was made, For 5.4, it would be necessary to test if the return is an array or an object, per example :
For information, I have this deprecation after update Orm to V3.1 directly when a form based on an entity was handled :
I will try to add a test. |
Thanks. Yes, we'd need this "is_array" test indeed on 5.4. |
I saw that, sorry :) |
I didn't, please add them and I'll take care of the rebase/retarget for 5.4 before merging. |
made it in #54271 |
set_error_handler(static function (int $errno, string $errstr) use (&$deprecation) { | ||
$deprecation[] = $errstr; | ||
}, E_USER_DEPRECATED); |
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.
We should not need a dedicated error handler for deprecations. Our error handler should make our tests fail if they trigger a deprecation. In other words: test the functionality, not the deprecation.
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.
so delete the test, the test for this fonctionnality already exists,
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 delete it
Closing in favor of #54271 then. |
…uessing field lengths (eltharin) This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [DoctrineBridge] Fix deprecation warning with ORM 3 when guessing field lengths | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT [DoctrineBridge] prevent deprecated message #54255 for symfony 5.4 Commits ------- 43d9c19 [DoctrineBridge] Fix deprecation warning with ORM 3 when guessing field lengths
prevent this deprecation :
User Deprecated: Using ArrayAccess on Doctrine\ORM\Mapping\FieldMapping is deprecated and will not be possible in Doctrine ORM 4.0. Use the corresponding property instead. (ArrayAccessImplementation.php:18 called by DoctrineOrmTypeGuesser.php:132, doctrine/orm#11211, package doctrine/orm)
same as symfony/doctrine-bridge#15