Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Allow null $parameters in Predicate::expression() #29

Merged
merged 2 commits into from
Sep 22, 2015
Merged

Allow null $parameters in Predicate::expression() #29

merged 2 commits into from
Sep 22, 2015

Conversation

HillTravis
Copy link
Contributor

Zend\Db\Sql\Predicate\Expression::__construct() allows nulls for the $valueParameter, but this method that passes the parameter through does not allow nulls, so you have to explicitly pass null. Instead, in Zend\Db\Sql\Predicate\Predicate::expression(), the $parameters parameter should allow nulls.

Zend\Db\Sql\Predicate\Expression::__construct() allows nulls for the $valueParameter, but this method that passes the parameter through does not allow nulls, so you have to explicitly pass null. Instead, in Zend\Db\Sql\Predicate\Predicate::expression(), the $parameters parameter should allow nulls.
@mwillbanks
Copy link
Contributor

@HillTravis can you create a unit test for this? I have actually had this issue before which I could see this being useful.

@HillTravis
Copy link
Contributor Author

@mwillbanks I added a unit test. I'm not sure why the Travis CI build is failing or why coverage would have decreased with the ADDITION of a unit test, but I ran the tests in my fork and everything passed.

@weierophinney
Copy link
Member

@HillTravis The failure is a coding standards check... and one that we just fixed on another PR. I'll review now.

Thanks!

@weierophinney weierophinney added this to the 2.5.2 milestone Sep 22, 2015
@weierophinney weierophinney self-assigned this Sep 22, 2015
@weierophinney weierophinney merged commit 51863b7 into zendframework:master Sep 22, 2015
weierophinney added a commit that referenced this pull request Sep 22, 2015
Allow null $parameters in Predicate::expression()
weierophinney added a commit that referenced this pull request Sep 22, 2015
weierophinney added a commit that referenced this pull request Sep 22, 2015
weierophinney added a commit that referenced this pull request Sep 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants