Promisify reverseDnsLookup #1235
Conversation
| .then(()=>{ | ||
| init(socket, client); | ||
| }) | ||
| .catch(e=>{ |
There was a problem hiding this comment.
I believe => should be surrounded with whitespaces. Once #1231 is merged, rebase on master, which I think should detect this :)
xPaw
force-pushed
the
xpaw/safer-auth-flow
branch
3 times, most recently
from
63a21f1
to
c412698
Compare
|
@metsjeesus, why do you still have xPaw's commit here and in your other PR? Also, are you up-to-date with master? |
|
Its only 1 method, reverseDnsLookup. Old code has some auth logic in it, what made it useless to use it anywhere else, example on #986 . Main point is, its cleaner to look and undestand what auth does, its not split up on many places. |
|
I'm gonna agree with @astorije for now. This doesn't provide any direct value. I will take another look at this after session stuff is redone. Login/session code is somewhat critical and subtle issue can easily slip in, so I will close this for now. Thanks for your help, I'll poke you on IRC on the next release. |
|
@metsjeesus, just so you don't take it the wrong way: thanks for your help, we do appreciate it a lot! Essentially, I believe this PR is doing 2 things: converting async stuff to using promises, and something else. In my understanding, this "something else" is the answer to the underlying problem, not the promisifying, but I'm not sure to understand what is the underlying problem. Just like @xPaw said, let's look at this again when the session stuff is ready, there is a lot of crossover right now and it gets confusing and tricky (because of critical component like @xPaw mentioned). |
So you can use the method somewhere else too, maybe on webirc on some extreme cases.