-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add CLIENT LIST command and fix CLIENT INFO #2368
Conversation
@K4ST0R Thanks for contributing (and not for the first time!) :) Looks like it's not working with 5.0 and 6.0, you can test it locally using
|
@leibale Thanks to you :) I fixed the issue with the old redis version. Question : maybe it is better to not validate the fields, they can vary quite a lot between the version ? |
@K4ST0R We have to make sure that the "supported versions" (5 - current) work as expected.. (sometimes it's not that fun or easy, but it has to be done..) |
Yes I totally understand! Everything is fine now, normally :) |
Codecov ReportBase: 95.85% // Head: 93.48% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2368 +/- ##
==========================================
- Coverage 95.85% 93.48% -2.38%
==========================================
Files 449 452 +3
Lines 4224 4280 +56
Branches 471 482 +11
==========================================
- Hits 4049 4001 -48
- Misses 107 209 +102
- Partials 68 70 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@K4ST0R sorry for the huge delay... Review my changes? 🙏 |
@leibale Don't worry! LGTM! :) Thanks to you. |
Description
Fix #1910
I have added the implementation of the following command to the project CLIENT LIST.
This PR also fix an issue with CLIENT INFO : new fields were added in new redis version, the regex was not extracting the good values.
Checklist
npm test
pass with this change (including linting)?