Skip to content
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

PHP 7.1 compatibility: harmonize method signatures #15

Merged
merged 1 commit into from
Apr 30, 2017
Merged

Conversation

xabbuh
Copy link
Owner

@xabbuh xabbuh commented Apr 28, 2017

No description provided.

@@ -196,7 +196,7 @@ public function getEncodings(array $filter = array())
/**
* {@inheritDoc}
*/
public function getEncodingsWithStatus($status, array $filter = array())
public function getEncodingsWithStatus($status, array $filter = null)
{
$filter['status'] = $status;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it work fine for null ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it works (see https://3v4l.org/p1lU1) at least for PHP versions <= 7.1. But I agree that this is not a very clean solution. So I pushed some more code making sure that $filter is an array before modifying it.

@stof
Copy link
Contributor

stof commented Apr 30, 2017

@xabbuh I suggest you to merge this PR and tag a 1.2.1 release fixing the compat with PHP 7.1, before doing the work of migrating to HTTPlug.

@xabbuh xabbuh merged commit 233e29b into master Apr 30, 2017
xabbuh added a commit that referenced this pull request Apr 30, 2017
This PR was merged into the 1.2.x-dev branch.

Discussion
----------

PHP 7.1 compatibility: harmonize method signatures

Commits
-------

233e29b PHP 7.1 compatibility: harmonize method signatures
@xabbuh xabbuh deleted the php-7.1 branch April 30, 2017 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants