Closed
Bug 1141621
Opened 10 years ago
Closed 9 years ago
Put shumway logging behind a pref
Categories
(Firefox Graveyard :: Shumway, defect)
Firefox Graveyard
Shumway
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: mossop, Assigned: MattN)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
We've had a general policy in the past that any logging in code should be controlled by a pref, default off unless there are particular reasons. This keeps console output as clean as possible for people running and debugging tests locally.
Can we put Shumway's logging behind a pref?
http://mxr.mozilla.org/mozilla-central/source/browser/extensions/shumway/chrome/ShumwayCom.jsm#49
http://mxr.mozilla.org/mozilla-central/source/browser/extensions/shumway/content/ShumwayUtils.jsm#38
http://mxr.mozilla.org/mozilla-central/source/browser/extensions/shumway/content/ShumwayStreamConverter.jsm#51
Flags: needinfo?(ydelendik)
Comment 1•10 years ago
|
||
Can we perhaps use shumway.debug.enabled for that?
Reporter | ||
Comment 2•10 years ago
|
||
Sounds good to me. You might also find it easier to use Preferences.jsm for prefs access rather than Services.prefs as you can define default values rather than needing try...catch blocks.
Comment 3•10 years ago
|
||
A logging pref doesn't need to block M3, our initial Nightly testing, because Shumway logging won't be as common because our whitelist will be pretty short.
Blocks: shumway-m4
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #3)
> A logging pref doesn't need to block M3, our initial Nightly testing,
> because Shumway logging won't be as common because our whitelist will be
> pretty short.
The logging in ShumwayUtils.jsm happens regardless of the size of the whitelist and happens on startup and shutdown which means it appears in all test logs.
Someone else can deal with the other logging which isn't as prevalent.
Comment 5•9 years ago
|
||
Comment on attachment 8612635 [details] [diff] [review]
v.1 Put ShumwayUtils.jsm logging behind shumway.debug
Review of attachment 8612635 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, Matt. The Shumway code in browser/extensions/shumway/ is actually a read-only copy of Shumway's upstream repo https://github.com/mozilla/shumway/.
Yury, can you please commit Matt's logging change (or something equivalent) before you land the next Shumway build on mozilla-central? I think these Shumway messages may be annoying other developers. :)
Attachment #8612635 -
Flags: review?(cpeterson) → feedback?(ydelendik)
Comment 6•9 years ago
|
||
Yury, can you pick this up?
Comment 7•9 years ago
|
||
Sending to Tobias because Yury is on PTO. :)
Flags: needinfo?(ydelendik) → needinfo?(schneider)
Comment 8•9 years ago
|
||
Comment on attachment 8612635 [details] [diff] [review]
v.1 Put ShumwayUtils.jsm logging behind shumway.debug
Review of attachment 8612635 [details] [diff] [review]:
-----------------------------------------------------------------
shumway.debug.enabled can be used for that.
Attachment #8612635 -
Flags: feedback?(ydelendik) → feedback+
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Yury Delendik (:yury) from comment #8)
> Comment on attachment 8612635 [details] [diff] [review]
> v.1 Put ShumwayUtils.jsm logging behind shumway.debug
>
> Review of attachment 8612635 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> shumway.debug.enabled can be used for that.
Are you going to make the pref name change and land this on GitHub? Or what are the next steps here?
Comment 10•9 years ago
|
||
You can change the pref on m-c, but open an issue at github to follow up.
Assignee | ||
Comment 11•9 years ago
|
||
Also switched to single quotes to match the file.
Attachment #8612635 -
Attachment is obsolete: true
Flags: needinfo?(tschneider)
Attachment #8715482 -
Flags: review?(ydelendik)
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → INCOMPLETE
Comment 12•9 years ago
|
||
Comment on attachment 8715482 [details] [diff] [review]
v.2 Put ShumwayUtils.jsm logging behind shumway.debug.enabled
Review of attachment 8715482 [details] [diff] [review]:
-----------------------------------------------------------------
The Shumway code was removed from the m-c.
Attachment #8715482 -
Flags: review?(ydelendik)
You need to log in
before you can comment on or make changes to this bug.
Description
•