Skip to content

check empty_data in choices of ChoiceType #25899

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

Closed
wants to merge 4 commits into from

Conversation

peter-gribanov
Copy link
Contributor

@peter-gribanov peter-gribanov commented Jan 23, 2018

Q A
Branch? bug fixes
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #25896
License MIT

If empty_data is callable, search it in choices. May be it is not a really callable data.

I don't exclude the probability of violation of BC.

@Simperfit
Copy link
Contributor

If it's a BC break then you should deprecate the behaviour for 4.1 and then do it for 5.0.

@xabbuh xabbuh added this to the 4.1 milestone Jan 24, 2018
@xabbuh xabbuh added the Form label Jan 24, 2018
@xabbuh
Copy link
Member

xabbuh commented Jan 24, 2018

As written in the referenced ticket you could deprecate the ability for a string to be handled as a callable (you can take a look at the 3.4 branch were we followed this approach for another option). However, the behaviour must not be changed here as that would be a BC break (in your own code you can use an anonymous function which just returns the string as a workaround).

@peter-gribanov
Copy link
Contributor Author

Is it too early to talk about a release 5.0?
And yes.
You suggest marking the use of handler as deprecated and at same time we have only one way to solve this bug, is to use a handler.
Are you seriously?

I certainly agree that a violation of BC is bad.
I don't mind marking this method as deprecated.
But i don't want to use the deprecated method to fix not my bug.

@xabbuh
Copy link
Member

xabbuh commented Jan 24, 2018

We know that Symfony 5.0 will be released in November 2019. And if we deprecate something in 4.1 to 4.4, we also know that we will remove this feature in 5.0.

I think we are talking about different things when talking about the deprecation. I do not want to deprecate the ability to use a callable entirely. What we should deprecate IMO is the possibility to pass a callable as a string as this can be ambiguous as you said. So in the future, if you want to make use of a callable, you would need to use an anonymous function.

@peter-gribanov
Copy link
Contributor Author

@xabbuh apparently i did not understand you.
You propose to mark use of handlers as a scalar value as deprecate, but to leave it possible to use anonymous function?

@xabbuh
Copy link
Member

xabbuh commented Jan 25, 2018

Exactly, you may want to take a look at #18020 where we did something similar in the past.

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.

4 participants