Make inputs optional #384
Make inputs optional #384
Conversation
|
Hey @Respect/troubleshooters, I'm really considering doing that for 1.0. It's the only BC break I would consider for the first major version (1.0). I really need your attention here. |
|
It is a pretty big BC break is it not? I'd rather have current API as is. On Sat, Aug 8, 2015 at 11:01 AM, Henrique Moody notifications@github.com
|
|
Yes it is, in the other hand that's how the library should work... |
|
This defeats the purpose of keeping a compatible API doesn't it? Although I am not against the idea, I am inclined to believe it defeats the purpose of keeping an old compatible API. On Sat, Aug 8, 2015 at 11:24 AM, Henrique Moody notifications@github.com
|
|
I don't see like that. It's really a huge BC break and it changes the way of creating rules indeed, but this PR keeps the same Concrete API we already have (discussed in #354). |
This interface should be used to identify rules which should always use the real validation without the optional input checking.
Make children of `AbstractRule` class to deal with optional values by creating an abstract method `validateConcrete()` which should contains the real validation and making `validate()`, which is now a final method, to check if the value is optional or not and then using the `validateConcrete()` method.
These changes was made using `sed`, but there are manual changes too. It also makes the classes below implement `RequiredValidatable`: - `AbstractComposite` - `AbstractWrapper` - `AlwaysInvalid` - `AlwaysValid` - `Each` - `NotEmpty` - `When`
|
Since there was no agreement on that, I closed this pull request. Any way, I'm not sure how we can proceed with the 1.0 version. I'm not really comfortable to release it as is specially because of the optional input problem. This pull request was only a alternative to the problem, but it's a very poor solution. Perhaps we should remove the optional input and just create the What I do know is that we have to release 1.0 on Oct 24 and there is a lot to talk about it until them and that also we have 2.0 to release but we don't even finished the discussions about the new Concrete API and I don't wanna make the decisions by myself, it's not how I wanna do that. |
2 participants
Fix #235