Closed Bug 1190452 Opened 5 years ago Closed 5 years ago

Remove dbg-server.jsm and use commonjs require to import DebuggerServer

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox43 fixed)

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Let's make a dedicated bug to get rid of dbg-server.jsm.
This jsm is useless as we should import this module via
   const { DebuggerServer } = require("devtools/server/main");
(Instead of Cu.import("resource://gre/modules/devtools/dbg-server.jsm", {}))

All usages of dbg-server.jsm prevent correctly reloading server files when using local source directory.
Attached patch patch v2Splinter Review
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7c7b7b7dd2c
This time, I'm not removing the jsm as it is used in too many addons.
Attachment #8642534 - Attachment is obsolete: true
... but I've dispatched some previous try run with dbg-server.jsm removed to verify there was no more usages. Some new one may slit through, we would have to verify during reviews and may be a bit latter, check m-c codebase for possible new usages of it.
Attachment #8644911 - Flags: review?(jryans)
Comment on attachment 8644911 [details] [diff] [review]
patch v2

Review of attachment 8644911 [details] [diff] [review]:
-----------------------------------------------------------------

Should dbg-server.jsm log a deprecation notice when it's used?
Attachment #8644911 - Flags: review?(jryans) → review+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> Comment on attachment 8644911 [details] [diff] [review]
> patch v2
> 
> Review of attachment 8644911 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Should dbg-server.jsm log a deprecation notice when it's used?

It's not a big deal if they keep using it or if we keep this alias.
But yes, we could do that. I would do that a bit later once file are moved, loader paths are cleaned up and everything is more stable as things might change again.
Btw, I've pushed two try just before landing.
One removing dbg-server.jsm:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cc2dd728d33
And another without removing it (what actually landed):
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=585e58baa292

Everything was green (enough)!
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/a50a0c78b557af3994fa78bbc8d830a3c77b6620
Bug 1190452 - Always import toolkit/devtools/server/main.js as CommonJS module. r=jryans
https://hg.mozilla.org/mozilla-central/rev/2fcd73fa8a7e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Depends on: 1198716
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.