-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add support for PUBSUB SHARDNUMSUB
#2541
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
Conversation
export function transformReply(rawReply: Array<string | number>): Record<string, number> { | ||
const transformedReply = Object.create(null); | ||
|
||
for (let i = 0; i < rawReply.length; i +=2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use entries()
to iterate over the array of records.. something idiomatic like:
for (const [channel, numSubscribers] of rawReply.entries()) {
transformedReply[channel] = numSubscribers;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every first factor of the iterator returned by the entries() is index, so I think it is not an appropriate use 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does "rawReply" look like ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://redis.io/commands/pubsub-shardnumsub/
Same as reply in pubsub numsub. Repeated channel(string) and count(number)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good, the tests need some changes..
Thanks for contributing! :)
}); | ||
|
||
testUtils.testWithClient('client.pubSubShardNumSub', async client => { | ||
assert.deepEqual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please make the test "non empty" so it'll actually test transforReply
as well?
PUBSUB SHARDNUMSUB
commandPUBSUB SHARDNUMSUB
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2541 +/- ##
==========================================
+ Coverage 95.70% 95.72% +0.02%
==========================================
Files 456 459 +3
Lines 4561 4543 -18
Branches 524 507 -17
==========================================
- Hits 4365 4349 -16
Misses 127 127
+ Partials 69 67 -2
☔ View full report in Codecov by Sentry. |
@david02324 @younes-io |
Description
very similar to the existing implementation of the PUBSUB NUMSUB command. :)
Checklist
npm test
pass with this change (including linting)?