| Submitter | Diff | Changes | Open Issues | Last Updated |
|---|---|---|---|---|
| Loading... | ||||
| Error loading review requests: | ||||
| Comment hidden (empty) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 4•5 months ago
|
||
| mozreview-review | ||
Comment on attachment 8862476 [details] Bug 1360237 - Fix eslint issues for devtools/client/framework/devtools.js. https://reviewboard.mozilla.org/r/134392/#review137342
|
|
||
Comment 5•5 months ago
|
||
| mozreview-review | ||
Comment on attachment 8862477 [details] Bug 1360237 - Make gDevToolsBrowser.inspectNode more readable by using async/await. https://reviewboard.mozilla.org/r/134394/#review137346 I have not read the dev-platform thread about using await/async, but this looks good to me. What was the outcome of the discussion? Can we use those more generally in our code now?
|
|
||
Comment 6•5 months ago
|
||
| mozreview-review | ||
Comment on attachment 8862478 [details] Bug 1360237 - Fix eslint issues for devtools/client/framework/devtools-browser.js. https://reviewboard.mozilla.org/r/134396/#review137348 Seems good to me. Since the files you changed in this series of commits are not clean, you should also make sure future eslint errors are caught in CI, but removing exceptions for these files in .eslintignore. ::: devtools/client/framework/devtools-browser.js:257 (Diff revision 1) > - (toolId == "webconsole" && toolbox.splitConsole))) > + (toolId == "webconsole" && toolbox.splitConsole))) { > - { > toolbox.fireCustomKey(toolId); > > - if (toolDefinition.preventClosingOnKey || toolbox.hostType == Toolbox.HostType.WINDOW) { > + if (toolDefinition.preventClosingOnKey || > + toolbox.hostType == Toolbox.HostType.WINDOW) { nit: I think we tend to vertically align conditions like this: ``` if (toolDefinition.preventClosingOnKey || toolbox.hostType == Toolbox.HostType.WINDOW) { ```
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
(Assignee) | |
Comment 10•5 months ago
|
||
(In reply to Patrick Brosset <:pbro> from comment #5) > Comment on attachment 8862477 [details] > Bug 1360237 - Makes gDevToolsBrowser.inspectNode more readable by using > async/await. > > https://reviewboard.mozilla.org/r/134394/#review137346 > > I have not read the dev-platform thread about using await/async, but this > looks good to me. What was the outcome of the discussion? Can we use those > more generally in our code now? It isn't really clear to me, but as I started receiving pathes with await/async from jryans (who's watching everyhing), I supposed we had a green light. But it is very confusing if you see these two messages: https://mail.mozilla.org/pipermail/firefox-dev/2017-March/005235.html https://mail.mozilla.org/pipermail/firefox-dev/2017-March/005233.html https://groups.google.com/forum/#!topic/mozilla.dev.platform/4--twpqTrMY Posted at about the same time. And it seems to be used in very various places: http://searchfox.org/mozilla-central/search?q=async+function&path= But when I see that: http://searchfox.org/mozilla-central/source/js/src/frontend/Parser.h#828-836 bool asyncIterationSupported() { #ifdef RELEASE_OR_BETA return false; #else I tend to think all this code using async/await is going to start throwing on merge to beta?! So, I think I will revert back to Task.
|
||
Comment 11•5 months ago
|
||
As posted in #devtools, async iteration is not the same as async / await. As far as I know, async / await was enabled for all channels in 52 (bug 1185106). Also, Firefox team is planning to rewrite the tree to async / await (bug 1353542).
| Comment hidden (mozreview-request) |
|
||
Comment 13•5 months ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac07cdd492c3 Fix eslint issues for devtools/client/framework/devtools.js. r=pbro https://hg.mozilla.org/integration/autoland/rev/30ea0bb33e98 Make gDevToolsBrowser.inspectNode more readable by using async/await. r=pbro https://hg.mozilla.org/integration/autoland/rev/a55f523231a2 Fix eslint issues for devtools/client/framework/devtools-browser.js. r=pbro
|
||
Comment 14•5 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/ac07cdd492c3 https://hg.mozilla.org/mozilla-central/rev/30ea0bb33e98 https://hg.mozilla.org/mozilla-central/rev/a55f523231a2
