Comment on attachment 8947288 [details] [diff] [review] release-13.patch Review of attachment 8947288 [details] [diff] [review]: ----------------------------------------------------------------- - Meta: - assigned, P3, enhancement, blocks meta: KO, added - commit message: OK - less than 20 commits: OK - TRY - try run green: KO 1 unreported intermittent (460 bytes leak on dt7 linux debug) - try run has rebuild: KO - Manual test - OK - README review - compare link: OK - commit link: OK - version number: OK - Properties file review - KO: wrong label, cherry pick bug? - New files - KO: remove mochi_log.txt ::: devtools/client/debugger/new/test/mochitest/browser.ini @@ +127,5 @@ > [browser_dbg-sourcemaps-reload.js] > [browser_dbg-sourcemaps-reloading.js] > [browser_dbg-sourcemaps2.js] > [browser_dbg-sourcemaps3.js] > +skip-if = true We now have 6 tests with skip-if = true for the new debugger, which is more 10% of our mochitests. This needs to be addressed. ::: devtools/client/locales/en-US/debugger.properties @@ +28,4 @@ > # context menu to set a directory as root directory > setDirectoryRoot.label=Set directory root > setDirectoryRoot.accesskey=r > +# LOCALIZATION NOTE (removeDirectoryRoot.label): This is the text that appears in the The rebase of my PR got rid of one blank line above this one? @@ +32,3 @@ > # context menu to remove a directory as root directory > removeDirectoryRoot.label=Remove directory root > +setDirectoryRoot.accesskey=d This is the wrong key? Looks like something went wrong with the cherry pick, since everything is OK on master :/

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b5e77f94ac993ff5264ba5fa963dbfe1dc608ee new patch coming

Comment on attachment 8947459 [details] [diff] [review] release-13-1.patch Review of attachment 8947459 [details] [diff] [review]: ----------------------------------------------------------------- LGTM but need to synchronize devtools/client/debugger/new/index.html first. ::: devtools/client/debugger/new/index.html @@ +19,5 @@ > <div id="mount"></div> > <script type="application/javascript" > src="chrome://devtools/content/shared/theme-switching.js"></script> > <script type="text/javascript"> > + const { BrowserLoader } = Components.utils.import("resource://devtools/client/shared/browser-loader.js", {}); Mass refactor from https://bugzilla.mozilla.org/show_bug.cgi?id=1431533, needs to be synced first.

Comment on attachment 8947482 [details] [diff] [review] release-13-2.patch Review of attachment 8947482 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Jason!

There are conflicts while trying to land, please take a look and provide an updated patch.

Yet again a mass refactor, from Bug 1339461.

small sanity try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=557dfb3b58b214027b55b730c723008d8c883d08

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f812e5fec015 Update Debugger Frontend v13. r=jdescottes