Skip to content

Fix regression, bring back socket support, fixes #950 #951

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

Closed
wants to merge 5 commits into from

Conversation

FGRibreau
Copy link

Fix regression, bring back socket support, fixes #950

@FGRibreau
Copy link
Author

Before this PR on master on my MBP (I don't know if master was stable or not):

1425 passing (2m)
8 pending
10 failing

after this PR:

1425 passing (2m)
8 pending
10 failing

no regression were introduced @BridgeAR. I will add some tests tomorrow morning :)

@FGRibreau
Copy link
Author

@BridgeAR https://coveralls.io/builds/4677286/source?filename=index.js#L1302 is not tested, but I will wait for your feedback. Should I implement createClient(socket, options) (which was not implemented before) or only keep the constructor implementation?

@FGRibreau
Copy link
Author

Hello @BridgeAR, any news on this? :) Do you want me to remove createClient(socket, options) implementation and only keep the constructor for backward compatibility?

@BridgeAR
Copy link
Contributor

@FGRibreau I'm really busy right now and I guess I won't find time before next monday :/

If you attend the FOSDEM we could meet there though.

@FGRibreau
Copy link
Author

Hello @BridgeAR,

This PR is too old so I will need to rebase, can you confirm you will be able to merge it then? I will only keep the constructor and remove the non-existing createClient(socket, options) :)

@@ -33,9 +33,21 @@ function handle_detect_buffers_reply (reply, command, buffer_args) {

exports.debug_mode = /\bredis\b/i.test(process.env.NODE_DEBUG);

function RedisClient (options) {
function RedisClient (globalOptions) {
Copy link
Author

Choose a reason for hiding this comment

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

function RedisClient (options, socket) {

@BridgeAR BridgeAR closed this in d858bd8 Mar 10, 2016
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.

RedisClient(stream, options) does not work anymore, reimplement it
2 participants