-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fix Subfilter support #156
base: 4.x
Are you sure you want to change the base?
Conversation
The original PR was from @jakejohns #131 . I spend quite sometime to make this happen. There are some glitches. But I am not sure how we can overcome that. If anyone have better ways we can consider that also. |
Anyone else interested in contributing can comment on this PR. I am happy to hear criticims. So no issues for raising the bar. |
@@ -365,19 +365,23 @@ protected function applySpec(Spec $spec, object $subject): bool | |||
* | |||
* | |||
*/ | |||
protected function failed(Spec $spec): Failure | |||
protected function failed(Spec $spec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one place I dislike.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is return void sufficient? The caller does not use the return value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier it was returning the Failure. As it currently changed this now can return Failure or FailureCollection. ie why return is removed. I will look once again if this returned value has some value inside the caller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is to add a failure. It seems no return value is needed.
Fix return type
Rename subfilter to subFilter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks good to me.
Co-authored-by: Akihito Koriyama <[email protected]>
Related to #155 .
Support maximum of 2.
RESULT :