[Config] type specific check for emptiness #15170
Conversation
xabbuh
commented
Jul 1, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #13736 |
| License | MIT |
| Doc PR |
|
👍 |
|
ping @symfony/deciders |
|
👍 |
1 similar comment
|
👍 |
|
I'd rather say, as also stated in the ticket, that So I'm +-0 with this solution. |
|
I thought about that too, but decided to go this way as throwing an exception would potentially break many existing applications. |
|
Then just change it in 2.8 or 3.0 |
|
Then I suggest to mark the method as deprecated in 2.8 and throw the exception in 3.0. Do you agree with this plan? |
|
@xabbuh I agree |
|
see #15285 for the deprecations |
|
I just discovered that the changes did not cover float nodes properly. I moved the check from |
|
Thank you @xabbuh. |
This PR was merged into the 2.3 branch. Discussion ---------- [Config] type specific check for emptiness | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #13736 | License | MIT | Doc PR | Commits ------- 0199fbf [Config] type specific check for emptiness
|
|
||
| public function getValidNonEmptyValues() | ||
| { | ||
| return array( |
There was a problem hiding this comment.
Well shouldn't that value be there as well ?
Maybe I'm wrong, I just wanted to make sure you didn't accidentally miss it :)
There was a problem hiding this comment.
It's true that we could have added it, but I think it's not absolutely necessary. Of course, if you think it deserves its own test, please don't hesitate to make a pull request for it. :)
Since Symfony 3.0 the cannotBeEmpty() is removed for an integerNode. For more information see: symfony/symfony#15170 symfony/symfony#15285