Skip to content

Conversation

@her-ur
Copy link

@her-ur her-ur commented Sep 19, 2017

No description provided.

@enumag
Copy link
Member

enumag commented Sep 19, 2017

According to the documentation the second argument is optional. https://github.com/phpredis/phpredis#script

@her-ur
Copy link
Author

her-ur commented Sep 19, 2017

Yes, you're right. I actually getting this warning on Debian with php5-redis 2.2.5-1:
Declaration of Kdyby\Redis\Driver\PhpRedisDriver::script($command, $script = NULL) should be compatible with Redis::script($cmd, ...$args)
Any thoughts what should i do with that? Thank you very much!

@enumag
Copy link
Member

enumag commented Sep 19, 2017

Oh it seems they updated the API with PHP Argument Unpacking (https://wiki.php.net/rfc/argument_unpacking). Which means you should change the function to something like this:

public function script($command, ...$args)
{
    return parent::script($command, ...$args);
}

Of course you would have to raise the required PHP version in composer.json to 5.6.

However this seems completely useless. The question here is why Kdyby overrides these methods in the first place since it would be equivalent if the methods were not there. Unfortunately I do not know this.

@enumag
Copy link
Member

enumag commented Sep 19, 2017

What happens if you simply remove the script method from Kdyby\Redis\Driver\PhpRedisDriver?

@her-ur
Copy link
Author

her-ur commented Sep 20, 2017

It is solution for me, but this method is defined also in IRedisDriver and that interface is used in Nette DI extension. So it would be BC break.

@jelen07
Copy link

jelen07 commented Nov 30, 2017

@her-ur Which DI extension use this interface?

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.

3 participants