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

Remove deprecated OptionsResolverInterface references from documentation #4565

Closed

Conversation

michaelperrin
Copy link
Contributor

OptionsResolverInterface has been deprecated in Symfony 2.6 in favor of OptionsResolver (see symfony/symfony#12156)

`OptionsResolverInterface` has been deprecated in Symfony 2.6 in favor of `OptionsResolver` (see symfony/symfony#12156)
@wouterj
Copy link
Member

wouterj commented Dec 1, 2014

👍

@michaelperrin
Copy link
Contributor Author

Well, I am not actually sure my PR should be merged, as I see that AbstractType still makes use of the OptionResolverInterface.
I am going to propose a PR on the Symfony repository to remove this reference.

@xabbuh
Copy link
Member

xabbuh commented Dec 1, 2014

@michaelperrin The reference can only be removed in 3.0 as it would breack BC otherwise.

@michaelperrin
Copy link
Contributor Author

@xabbuh This is what I was thinking about, many libraries would get broken as PHP would raise an error on the declaration of setDefaultOptions.

Well, this PR was not a good idea! Before closing it, I have a question. Is the master branch of symfony-docs repository corresponding to the docs of Symfony 3.0?
In that case, this PR can be reopened on the master branch in case OptionsResolverInterface is removed from the master branch of Symfony as well.

@xabbuh
Copy link
Member

xabbuh commented Dec 1, 2014

@michaelperrin Yes, branches of the documentation and the code repositories are the same.

@xabbuh
Copy link
Member

xabbuh commented Dec 1, 2014

see also symfony/symfony#12782

@michaelperrin
Copy link
Contributor Author

Alright then, I think i can propose a PR on the master branch of Symfony that removes the OptionsResolverInterface (Hi @webmozart !) and then this PR will make sense.

It seems that the removal of deprecated stuff on Symfony 3 hasn't started yet, but maybe it's already time to prepare it.

@stof
Copy link
Member

stof commented Dec 1, 2014

It seems that the removal of deprecated stuff on Symfony 3 hasn't started yet, but maybe it's already time to prepare it.

Actually, it has already started

@michaelperrin
Copy link
Contributor Author

Ok then, so I can do it on the OptionsResolverInterface

@peterrehm
Copy link
Contributor

As I pointed this out in the symfony repository I think we should find a way to have an upgrade path without depreciation notices on the form types as well.

There are several possible approaches I can think of, but I think we should await feedback from @webmozart how he wants to have it. Currently in the master branch relies still on the interface.

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/AbstractType.php#L45

@weaverryan
Copy link
Member

Thanks for opening this (and @peterrehm for the issue on the core code). This is indeed awkward! For now, I'm going to wait for the core solution to this. As far as the docs go, if I understand everything correctly, the only thing that we can recommend that is functional is the deprecated OptionsResolverInterface, correct?

If I'm mistaken and there are some changes that can be made now, let me know!

Thanks!

@peterrehm
Copy link
Contributor

I have just provided an PR which might resolve that issue, but I think it can only be resolved for 2.7, even though I think it should go into 2.6. (symfony/symfony#12891)

Currently I do not see any workaround on this behavior.

weaverryan added a commit that referenced this pull request Jan 21, 2015
…method (peterrehm)

This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #4786).

Discussion
----------

Replaced setDefaultOptions by the new configureOptions method

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | yes|no (PR symfony/symfony#12891)
| Applies to    | 2.7
| Fixed tickets | #4565

Pending on the merge of symfony/symfony#12891.

With this PR all references to setDefaultOptions and the deprecated OptionResolverInterface are removed in 2.7.

Commits
-------

728205f Replaced setDefaultOptions by the new configureOptions method
@weaverryan weaverryan closed this Jan 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants