Closed
Bug 1397296
Opened 7 years ago
Closed 7 years ago
[nosdk] remove loader paths mapped to addon-sdk resources
Categories
(DevTools :: General, enhancement, P1)
DevTools
General
Tracking
(firefox57 fixed)
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
Details
(Whiteboard: [reserve-nosdk])
Attachments
(1 file)
We define the following paths in both Loader.jsm and the worker loader:
> "": "resource://gre/modules/commonjs/"
This path normally maps to addon sdk resources but we shouldn't use it anymore.
Not blocking though.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=47c4c803895b48347c683aeaacd63e5e9367740c
Assignee | ||
Comment 3•7 years ago
|
||
Requested for feedback on the mailing list, as specified in the loader files comments: https://groups.google.com/d/msg/mozilla.dev.developer-tools/Kfm-Pdb_36U/8D9A1SSMCwAJ
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8905064 [details] Bug 1397296 - remove mappings for gre/modules/commonjs in devtools loaders; https://reviewboard.mozilla.org/r/176874/#review181852 I imagine you would be asking me for review. Yes, we should do that, assuming it doesn't undercover yet other SDK leftovers. Note that the note to warn on the devtools mailing list was mostly for when you introduce a new mapping, as we are trying to lower the number of special mappings. I'm wondering if we can then simplify the mapping story in the loader which takes precious milliseconds for each require... http://searchfox.org/mozilla-central/source/devtools/shared/base-loader.js#314-349
Attachment #8905064 -
Flags: review+
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #4) > Comment on attachment 8905064 [details] > Bug 1397296 - remove mappings for gre/modules/commonjs in devtools loaders; > > https://reviewboard.mozilla.org/r/176874/#review181852 > > I imagine you would be asking me for review. > Yes, we should do that, assuming it doesn't undercover yet other SDK > leftovers. Thanks for the review! Try seems happy so far: https://treeherder.mozilla.org/#/jobs?repo=try&revision=47c4c803895b48347c683aeaacd63e5e9367740c&selectedJob=128933902 > > Note that the note to warn on the devtools mailing list was mostly for when > you introduce a new mapping, > as we are trying to lower the number of special mappings. Ok good to know :) I doubt anyone will complain but with "// ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠" spammed on every other line in the area, I preferred to ask. I still wait until tomorrow before landing, just in case. > > > I'm wondering if we can then simplify the mapping story in the loader which > takes precious milliseconds for each require... > http://searchfox.org/mozilla-central/source/devtools/shared/base-loader. > js#314-349 We should definitely have a follow-up for that.
Updated•7 years ago
|
Flags: qe-verify-
Priority: -- → P1
Target Milestone: --- → Firefox 57
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bd760aeac6c0 remove mappings for gre/modules/commonjs in devtools loaders;r=ochameau
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bd760aeac6c0
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•