Bug 1027232 - Restart b2g #2
Conversation
| sockit.connect({ host: '127.0.0.1', port: port }); | ||
|
|
||
| var s = sockit.read(16).toString(); | ||
| console.error('read', s); |
There was a problem hiding this comment.
Why is console.error() used here instead of console.debug() or console.log()? This doesn't look like an error case...
There was a problem hiding this comment.
they are actually left over debug I have removed here... BOTH.
| // The marionette client should do that for you. | ||
| // https://bugzilla.mozilla.org/show_bug.cgi?id=1033402 | ||
| // Callback still needs to be called Asynchronously | ||
| setTimeout(callback, 0); |
There was a problem hiding this comment.
Merely an FYI here: you could leave off the 0 here, or since it's Node, you can use the much more efficient process.nextTick(callback).
There was a problem hiding this comment.
I'd like to avoid Node specific stuff if I can :-)
I have seen setTimeout(callback, 0) elsewhere. MDN doesn't tell us that the delay is an optional parameter.
There was a problem hiding this comment.
If we just call the callback, we should be good since spawn is async.
There was a problem hiding this comment.
Nope it doesn't work if I just call the callback. I actually tried that. I didn't dig the problem but the setTimeout did the trick.
|
@gaye I have updated the PR addressing comments. |
|
@gaye this is a critical blocking issue. Please prioritize doing this review we need this asap. |
| @@ -62,7 +62,18 @@ Host.prototype = { | |||
| 'tcp:' + DEFAULT_PORT]); | |||
| adb.on('close', function() { | |||
| debug('Set adb forward to ' + port); | |||
There was a problem hiding this comment.
debug code. because it is useful when you need to diagnose. not sure what your question is.
There was a problem hiding this comment.
Sorry I didn't see that this was called after the adb port forward command finished
Bug 1027232 - Restart b2g r=gaye
No description provided.