In the continuation of bug 1227474, DebuggerClient.connect is one of the last, if not the last one? to not return a promise and requires creating temporary promise when called from a Task.

I replaced all the obvious occurances of client.connect to use promises. So that we stop using the callback paradigm, even when copy pasting existing code. Also, in complex code, instead of having code with a lot of indentation like method().then(() => { promise().then(() => { }); }); We will do: method() .then(() => promise()) .then(() => { }); It will make it easier to switch to Task if needed.

Comment on attachment 8712225 [details] [diff] [review] patch v3 Review of attachment 8712225 [details] [diff] [review]: ----------------------------------------------------------------- Great, thanks! Please also clean up: 1. https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/connect/connect.js#83 2. https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/toolbox-process-window.js#36 3. All cases of "gClient.connect": https://dxr.mozilla.org/mozilla-central/search?q=gClient.connect+path%3Adevtools&redirect=false&case=true ::: devtools/shared/client/main.js @@ +334,5 @@ > * > * @param aOnConnected function > * If specified, will be called when the greeting packet is > * received from the debugging server. > */ Document the returned promise above.

I'll merge this patch with the previous one. Here is just the doc for DebuggerClient.connect. Could you r+ patch v3 with this on top of it?

I'll also merge this patch once r+. Waiting for green try before r?.

Fixed the webapps test failure. debugger and server tests would benefit some other refactoring, as they have a lot of initialization code duplicated. But I'm not sure it is a good idea to refactor this in this bug? I think it would be great to first convert them to add_task.

Comment on attachment 8712811 [details] [diff] [review] convert gClient.connect - v2 Review of attachment 8712811 [details] [diff] [review]: ----------------------------------------------------------------- Great, thanks again for even more cleanup! ::: devtools/client/framework/connect/connect.js @@ +77,5 @@ > let response = yield clientConnect(); > yield onConnectionReady(...response); > }); > > function clientConnect() { Inline this function where it's called.

Comment on attachment 8712225 [details] [diff] [review] patch v3 r+ given the additional patches.

https://hg.mozilla.org/integration/fx-team/rev/f83987c2da14dde169084ed35a7c273953be6653 Bug 1239276 - Convert DebuggerClient.connect to promise. r=jryans

Squashed and rebased.

https://hg.mozilla.org/integration/fx-team/rev/5cb797a911407cbddecd4f0c347ca0da251cff7a Bug 1239276 - Add missing parenthese in test_protocol_async.js r=me. CLOSED TREE

https://hg.mozilla.org/mozilla-central/rev/f83987c2da14 https://hg.mozilla.org/mozilla-central/rev/5cb797a91140