Closed
Bug 1072625
Opened 10 years ago
Closed 10 years ago
Cleanup debug output in AdbController
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.1 S5 (26sep)
People
(Reporter: dhylands, Assigned: dhylands)
References
Details
Attachments
(1 file, 1 obsolete file)
9.77 KB,
patch
|
fabrice
:
review+
|
Details | Diff | Splinter Review |
Currently, AdbController uses: > if (this.DEBUG) { > this.debug("some string"); > } it would be cleaner if it just did: > debug("some string");
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8494806 -
Flags: review?(fabrice)
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 2.1 S5 (26sep)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → dhylands
Comment 2•10 years ago
|
||
Comment on attachment 8494806 [details] [diff] [review] Cleanup up debug usage in AdbController Review of attachment 8494806 [details] [diff] [review]: ----------------------------------------------------------------- We tend to just do: const DEBUG = false; function debug(str) { ... } and then: DEBUG && debug("something happened"); This guarantees that we don't eval the debug() parameters when we don't need too. This file doesn't have any performance sensitive logging but if you don't mind that would be nice to use this style. ::: b2g/chrome/content/devtools/adb.js @@ +7,5 @@ > "use strict"; > > // This file is only loaded on Gonk to manage ADB state > > +var DEBUG = false; s/var/const
Attachment #8494806 -
Flags: review?(fabrice)
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8494806 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8496376 -
Flags: review?(fabrice)
Comment 4•10 years ago
|
||
Comment on attachment 8496376 [details] [diff] [review] Cleanup up debug usage in AdbController v2 Review of attachment 8496376 [details] [diff] [review]: ----------------------------------------------------------------- r=me ::: b2g/chrome/content/devtools/adb.js @@ +204,4 @@ > try { > libcutils.property_set("persist.sys.usb.config", newConfig); > } catch(e) { > + console.error("Error configuring adb: " + e); does that really work? We usually do Cu.reportError(...) in chrome code.
Attachment #8496376 -
Flags: review?(fabrice) → review+
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/9fe9bace9ed5
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9fe9bace9ed5
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•