Closed Bug 1072625 Opened 5 years ago Closed 5 years ago

Cleanup debug output in AdbController

Categories

(Firefox OS Graveyard :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S5 (26sep)

People

(Reporter: dhylands, Assigned: dhylands)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently, AdbController uses:

> if (this.DEBUG) {
>  this.debug("some string");
> }

it would be cleaner if it just did:

> debug("some string");
Attachment #8494806 - Flags: review?(fabrice)
Target Milestone: --- → 2.1 S5 (26sep)
Assignee: nobody → dhylands
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)
Attachment #8494806 - Attachment is obsolete: true
Attachment #8496376 - Flags: review?(fabrice)
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+
https://hg.mozilla.org/mozilla-central/rev/9fe9bace9ed5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Depends on: 1084955
You need to log in before you can comment on or make changes to this bug.