[Form] [Hackday] [2.7] Add a deprecation note about VirtualFormAwareIterator #12651
Conversation
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | #12604 |
| License | MIT |
| Doc PR |
| @@ -30,6 +30,10 @@ class VirtualFormAwareIterator extends \IteratorIterator implements \RecursiveIt | |||
| */ | |||
| public function getChildren() | |||
| { | |||
| if ('Symfony\Component\Form\Util\VirtualFormAwareIterator' === get_class()) { | |||
| trigger_error('class VirtualFormAwareIterator is deprecated since version 2.7 and will be removed in 3.0. Use InheritDataAwareIterator instead.', E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
please move the notice in the constructor (but do not forget to call the parent)
| parent::__construct($iterator); | ||
|
|
||
| if ('Symfony\Component\Form\Util\VirtualFormAwareIterator' === get_class()) { | ||
| trigger_error('class VirtualFormAwareIterator is deprecated since version 2.7 and will be removed in 3.0. Use InheritDataAwareIterator instead.', E_USER_DEPRECATED); |
There was a problem hiding this comment.
We discoevered a better way to trigger the notice: below the namespace declaration, see https://github.com/symfony/symfony/pull/12739/files
(btw, I don't see the case for the if check, can you remove it or justify it please?)
There was a problem hiding this comment.
why not throwing the trigger_error in the constructor and not after the namespace declaration, like: https://github.com/symfony/symfony/pull/12739/files ?
There was a problem hiding this comment.
Adding it after the namespace declaration means that it is only triggered once when the class is loaded and not every time an instance of it is created.
There was a problem hiding this comment.
@nicolas-grekas The reason about if is because class InheritDataAwareIterator extends of this class
There was a problem hiding this comment.
@nicolas-grekas putting it when loading the file does not trigger a warning each time the class is instantiated (and could trigger it when not needed). We need to trigger the error only when the deprecated feature is used (which may not even be at instantiation time if it is always instantiated for BC reasons)
|
Cherry-picked in #12968, thanks |
…cmorales) This PR was merged into the 2.7 branch. Discussion ---------- Deprecations | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #12625, #12605, #12628, #12622, #12642, #12609, #12651, #12604, #12607, #12667, #12648 | License | MIT | Doc PR | - Cherry-picking some pending PRs to make them move forward Commits ------- badf8fc [Form] Log deprecation of constants, fixes #12607 #12667 1d58df4 Fix deprecation notice on VirtualFormAwareIterator e2a19ee Add a deprecation note about VirtualFormAwareIterator ab4d9b8 Add a deprecation note about CsrfProviderInterface cb70632 [HttpKernel] fix deprecation notice for Kernel::init() b5a315d [HttpKernel] Added deprecated error to init() 70012c1 [Hackday] [2.7] Add a deprecation note about TypeTestCase