Skip to content

Conversation

minororange
Copy link

redis connection instance can be custom made,such as Swoole program

see: https://github.com/hyperf/metric/blob/master/src/Adapter/Prometheus/Redis.php

Signed-off-by: yangchenzhuo <yangchenzhuo@icsoc.net>
*/
public static function fromExistingConnection(\Redis $redis): self
public static function fromExistingConnection($redis): self
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the explicit type here?

Copy link
Author

Choose a reason for hiding this comment

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

yes, in Hyperf program,Redis instance is not \Redis,but functions are same.

Choose a reason for hiding this comment

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

By looking at the documentation it states hyperf returns a proxy object to \Redis: https://hyperf.wiki/3.1/#/en/redis?id=usage

Looks like the implementation, is still typeable by using \Hyperf\Redis\Redis. Would you mind adding that type side-by-side \Redis|\Hyperf\Redis\Redis?

Union types where added in php 8.0, this library still supports 7.2. @LKaemmerling do you mind dropping support for outdated and unmaintained php versions, too? I'd like to create a PR right away.

Copy link
Member

@LKaemmerling LKaemmerling left a comment

Choose a reason for hiding this comment

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

Hey, thank you for your contribution. I have one small suggested change/question about a change!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants