-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add support for Redis 6 auth pass [user]
#1508
Conversation
@Salakar can you please review it? |
@stockholmux I don't see, what am I missing? |
@gkorland Any updates on this? |
@Salakar can you please review this PR? |
@gkorland Looks good to me. |
@Salakar can you please review this PR? |
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.
LGTM
Any updates on this? |
Will this ever be merged? |
Not yet, it will be in the next couple if days |
@@ -61,7 +61,7 @@ describe('client authentication', function () { | |||
}); | |||
var tmp = client.command_queue.get(0).callback; | |||
client.command_queue.get(0).callback = function (err, res) { | |||
client.auth = function (pass, callback) { | |||
client.auth = function (pass, user, callback) { |
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.
@leibale shouldn't we keep the original test too? for backward?
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.
@gkorland overriding a function on the client is not part of the API..
@leibale why does the lint fail? |
@gkorland IDK whats doing on, it seems like it's running old workflows |
problem related to redis/node-redis#1508
pending NodeRedis/redis-commands#35