Skip to content

Conversation

@ybensacq
Copy link
Contributor

@ybensacq ybensacq commented Feb 8, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #17701
License MIT

-----

* deprecated the "choices_as_values" option of ChoiceType
* deprecated support for Traversable in `ResizeFormListener::PreSubmit` method
Copy link
Contributor

Choose a reason for hiding this comment

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

the method is called preSubmit without the uppercase.

$form = $event->getForm();
$data = $event->getData();

if ($data instanceof \Traversable){
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing space before the curly brace and IMO this should be $data instanceof \Traversable && $data instanceof \ArrayAccess because if someone pass an object that implements Traversable but not ArrayAccess the deprecation will be thrown but the $data will be reseted to an empty array.

$form = $event->getForm();
$data = $event->getData();

if ($data instanceof \Traversable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @ybensacq seems you missed to fix one of @dosten comment here.

Should be if ($data instanceof \Traversable && $data instanceof \ArrayAccess)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HeahDude it's ok in my last commit :
ybensacq@554307c

Copy link
Member

Choose a reason for hiding this comment

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

you updated only one of the two places in that commit

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, tthe condition should be:

if ($data instanceof \Traversable && $data instanceof \ArrayAccess) {
    @trigger_error('Support for Traversable is deprecated since version 3.1 and will be removed in 4.0. You should not use it anymore.', E_USER_DEPRECATED);
}

@netickfr
Copy link

netickfr commented Feb 9, 2016

Thanks Yannick :)

@xabbuh
Copy link
Member

xabbuh commented Feb 9, 2016

should also be documented in the upgrade files

@dosten
Copy link
Contributor

dosten commented Feb 9, 2016

@HeahDude I think that do not worth it because at the moment of remove deprecated features I search for "trigger_error" or "E_USER_DEPRECATED" for example.

@ybensacq
Copy link
Contributor Author

ybensacq commented Feb 9, 2016

Ok thanks, all is ok now.

@HeahDude
Copy link
Contributor

HeahDude commented Feb 9, 2016

@dosten yes and then you have to search the code to remove or edit, so maybe a comment can be useful at this time.

Besides that 👍

$data = $event->getData();

if ($data instanceof \Traversable && $data instanceof \ArrayAccess) {
@trigger_error('Support for objects implementing both \Traversable and \ArrayAccess is deprecated since version 3.1 and will be removed in 4.0.Use an array instead.', E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing space after the dot: will be removed in 4.0.Use an array

@dosten
Copy link
Contributor

dosten commented Feb 9, 2016

👍

Status: Reviewed

@fabpot
Copy link
Member

fabpot commented Feb 18, 2016

Thank you @ybensacq.

@fabpot fabpot closed this Feb 18, 2016
fabpot added a commit that referenced this pull request Feb 18, 2016
…method ResizeFormListener::PreSubmit (ybensacq)

This PR was squashed before being merged into the 3.1-dev branch (closes #17732).

Discussion
----------

[DEPRECATION] : deprecated support for Traversable in method ResizeFormListener::PreSubmit

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #17701
| License       | MIT

Commits
-------

68c9305 [DEPRECATION] : deprecated support for Traversable in method ResizeFormListener::PreSubmit
@fabpot fabpot mentioned this pull request May 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants