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

Hotfix #285 #287

Merged
merged 2 commits into from Nov 30, 2017

Conversation

Projects
None yet
3 participants
@webimpress
Contributor

webimpress commented Nov 30, 2017

I've checked and changing to else is good enough solution. Even if we have there non-string type it's gonna work as before, see:
https://wiki.php.net/rfc/counting_non_countables\

Calling count() on a scalar or object that doesn't implement the Countable interface returns 1.

Removed also part of the condition to check if variable is instance of Countable. It is not possible in that place. Please note the condition was wrong and not even working because Countable was not imported.

Resolves #285

/cc @ezimuel @remicollet

webimpress added some commits Nov 30, 2017

Change elseif to else
It bring back the previous functionality
https://wiki.php.net/rfc/counting_non_countables

Calling `count()` on a scalar or object that doesn't implement
the Countable interface returns 1.
(This behaviour has been changed in PHP 7.2)
Removed unused condition part
It is not possible to have there Countable element.
Please note it was not even imported so it was not working at all.
@shaunfreeman

This comment has been minimized.

Show comment
Hide comment
@shaunfreeman

shaunfreeman Nov 30, 2017

I was getting this warning message in PHP 7.2
Warning: count(): Parameter must be an array or an object that implements Countable in /vendor/zendframework/zend-db/src/Sql/AbstractSql.php on line 244
When using a reultset with Zend\DB\Resultset\Resultset this hotfix fixes this issue.

shaunfreeman commented Nov 30, 2017

I was getting this warning message in PHP 7.2
Warning: count(): Parameter must be an array or an object that implements Countable in /vendor/zendframework/zend-db/src/Sql/AbstractSql.php on line 244
When using a reultset with Zend\DB\Resultset\Resultset this hotfix fixes this issue.

@webimpress

This comment has been minimized.

Show comment
Hide comment
@webimpress

webimpress Nov 30, 2017

Contributor

@shaunfreeman the fix will be released with 2.9.0, probably soon :)

Contributor

webimpress commented Nov 30, 2017

@shaunfreeman the fix will be released with 2.9.0, probably soon :)

@ezimuel ezimuel merged commit 366661f into zendframework:master Nov 30, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.008%) to 47.736%
Details

@webimpress webimpress deleted the webimpress:hotfix/285 branch Nov 30, 2017

@ezimuel

This comment has been minimized.

Show comment
Hide comment
@ezimuel

ezimuel Nov 30, 2017

Member

@webimpress thanks! @shaunfreeman we'll release 2.9.0 next week.

Member

ezimuel commented Nov 30, 2017

@webimpress thanks! @shaunfreeman we'll release 2.9.0 next week.

ezimuel added a commit that referenced this pull request Nov 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment