{"action":"created","issue":{"url":"https://api.github.com/repos/NodeRedis/node_redis/issues/894","labels_url":"https://api.github.com/repos/NodeRedis/node_redis/issues/894/labels{/name}","comments_url":"https://api.github.com/repos/NodeRedis/node_redis/issues/894/comments","events_url":"https://api.github.com/repos/NodeRedis/node_redis/issues/894/events","html_url":"https://github.com/NodeRedis/node_redis/pull/894","id":112181912,"number":894,"title":"Add support for TLS connections","user":{"login":"paddybyers","id":1137574,"avatar_url":"https://avatars.githubusercontent.com/u/1137574?v=3","gravatar_id":"","url":"https://api.github.com/users/paddybyers","html_url":"https://github.com/paddybyers","followers_url":"https://api.github.com/users/paddybyers/followers","following_url":"https://api.github.com/users/paddybyers/following{/other_user}","gists_url":"https://api.github.com/users/paddybyers/gists{/gist_id}","starred_url":"https://api.github.com/users/paddybyers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/paddybyers/subscriptions","organizations_url":"https://api.github.com/users/paddybyers/orgs","repos_url":"https://api.github.com/users/paddybyers/repos","events_url":"https://api.github.com/users/paddybyers/events{/privacy}","received_events_url":"https://api.github.com/users/paddybyers/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-10-19T16:26:18Z","updated_at":"2015-10-23T12:35:44Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/NodeRedis/node_redis/pulls/894","html_url":"https://github.com/NodeRedis/node_redis/pull/894","diff_url":"https://github.com/NodeRedis/node_redis/pull/894.diff","patch_url":"https://github.com/NodeRedis/node_redis/pull/894.patch"},"body":"This PR supersedes https://github.com/NodeRedis/node_redis/pull/527, with those changes being rebased to the current `master`.\r\n\r\nI have add tests in `tls.spec.js` which runs with the variants of `options`, `ip` of `allTests`. This seemed the most direct way to add tests, although you will see that there is a lot of overlap with the `connection.spec.js` tests. I wondered if instead TLS should be a different set of `options` or a different `protocol` enumerated by `allTests` but that looked like it would generate much greater redundancy.\r\n\r\nCurrently the TLS tests run unconditionally but we might want a way to skip them if `stunnel` isn't available on the platform running the tests.\r\n\r\nI have run the tests locally only, on a Mac, so YMMV on other platforms. Feedback appreciated."},"comment":{"url":"https://api.github.com/repos/NodeRedis/node_redis/issues/comments/150559710","html_url":"https://github.com/NodeRedis/node_redis/pull/894#issuecomment-150559710","issue_url":"https://api.github.com/repos/NodeRedis/node_redis/issues/894","id":150559710,"user":{"login":"BridgeAR","id":8822573,"avatar_url":"https://avatars.githubusercontent.com/u/8822573?v=3","gravatar_id":"","url":"https://api.github.com/users/BridgeAR","html_url":"https://github.com/BridgeAR","followers_url":"https://api.github.com/users/BridgeAR/followers","following_url":"https://api.github.com/users/BridgeAR/following{/other_user}","gists_url":"https://api.github.com/users/BridgeAR/gists{/gist_id}","starred_url":"https://api.github.com/users/BridgeAR/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BridgeAR/subscriptions","organizations_url":"https://api.github.com/users/BridgeAR/orgs","repos_url":"https://api.github.com/users/BridgeAR/repos","events_url":"https://api.github.com/users/BridgeAR/events{/privacy}","received_events_url":"https://api.github.com/users/BridgeAR/received_events","type":"User","site_admin":false},"created_at":"2015-10-23T12:35:44Z","updated_at":"2015-10-23T12:35:44Z","body":"I'm sorry that I was not able to do a proper review yet. I only had a short glimpse yet. I'm really busy at the moment.\r\n\r\nPlease include stunnel for travis ([example](https://github.com/isotoma/isotoma.recipe.stunnel/blob/master/.travis.yml)). We'll probably have to deactivate the the tests for windows though. You can do this just the same we did with the redis process (`helper.redisProcess().spwanFailed()`).\r\n\r\nOne style nit: please always use a space between an if and a bracket.\r\n\r\nI think it's already looking very promising and I'm excited that after over three years[sic!] this is going to be included in node_redis :) And I'm also very happy that you were still willing to code this.\r\n\r\nAfter looking in the code it might be worth to deprecate .createClient() and just use new Redis(options) instead. What do you think? That way it might become even cleaner, but this should be another pull request."}}