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

Adds prefix support for redis-subscriber #116

Merged
merged 2 commits into from
Jul 4, 2022
Merged

Adds prefix support for redis-subscriber #116

merged 2 commits into from
Jul 4, 2022

Conversation

huangdijia
Copy link
Contributor

This PR is adds prefix support to compatible with redis connection.

$sub = new \Mix\Redis\Subscriber\Subscriber('127.0.0.1', 6379, '', 5, 'my_prefix:');

@onanying
Copy link
Member

onanying commented Jul 3, 2022

@huangdijia 我感觉前缀这种功能应该业务层去做

@huangdijia
Copy link
Contributor Author

@huangdijia 我感觉前缀这种功能应该业务层去做

phpredis自带前缀功能,如果redis-subscriber也支持,那就可以复用配置了。

@huangdijia
Copy link
Contributor Author

hyperf/hyperf#4892

@onanying
Copy link
Member

onanying commented Jul 3, 2022

phpredis 也是以 option 的方式配置的,没有在构造里面 https://github.com/phpredis/phpredis#example-7 ,我真的感觉这个应该放到业务中

@huangdijia
Copy link
Contributor Author

phpredis 也是以 option 的方式配置的,没有在构造里面 https://github.com/phpredis/phpredis#example-7 ,我真的感觉这个应该放到业务中

那改一下,加一个setPrefix()方法,这样就不影响构造方法了

@huangdijia
Copy link
Contributor Author

phpredis 起码也有个option可以设置,没让用户自己在业务逻辑里实现😂

@onanying
Copy link
Member

onanying commented Jul 3, 2022

@huangdijia 属性赋值吧,我改了一下

@huangdijia
Copy link
Contributor Author

@huangdijia 属性赋值吧,我改了一下

直接对public属性赋值?也可以吧,我看mix-php比较喜欢用public🤪

@onanying
Copy link
Member

onanying commented Jul 3, 2022

因为我总是觉得 SetFoo 和 直接 ->$foo 区别不大,大部分情况,小部分情况又不会用 Set 这种名字命名,可能我写go写多了导致的吧

@huangdijia
Copy link
Contributor Author

因为我总是觉得 SetFoo 和 直接 ->$foo 区别不大,大部分情况,小部分情况又不会用 Set 这种名字命名,可能我写go写多了导致的吧

PHP跟go不太一样,PHP默认是弱类型,且mix-php目前这个版本也是没有强制类型,万一传入的不是string,可能导致错误甚至崩溃。

@onanying onanying merged commit 3545744 into mix-php:master Jul 4, 2022
@onanying
Copy link
Member

onanying commented Jul 4, 2022

@huangdijia huangdijia deleted the adds-prefix-support branch July 14, 2022 15:59
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.

2 participants