Closed Bug 1071870 Opened 6 years ago Closed 6 years ago

Log.Level.All doesn't work with some /services/ modules since they assume valid log levels are truthy

Categories

(Cloud Services :: General, defect)

defect
Not set
Points:
1

Tracking

(Not tracked)

RESOLVED FIXED
mozilla35
Iteration:
35.2

People

(Reporter: MattN, Assigned: MattN)

Details

Attachments

(1 file, 1 obsolete file)

Log.jsm's Log.Level.All === 0 which is kinda unfortunate as it leads to bugs like this one. We have some modules that were doing the following and therefore fail when Log.Level.All is specified:
  log.level = Log.Level[level] || Log.Level.Error;
Attachment #8494003 - Flags: review?(mhammond)
Flags: qe-verify-
Flags: firefox-backlog+
As discussed on IRC with markh and Unfocused, switching All to -1 will get rid of the footgun.

TBH I'm not sure why there are two mapping from Level to number but I think this should be fine.
Attachment #8494003 - Attachment is obsolete: true
Attachment #8494003 - Flags: review?(mhammond)
Attachment #8494227 - Flags: review?(mhammond)
Comment on attachment 8494227 [details] [diff] [review]
v.2 Fix the problem at the source (All === 0)

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

LGTM, although I'd be inclined to add a comment to the first map to briefly say we don't want .All to be falsey.
Attachment #8494227 - Flags: review?(mhammond) → review+
https://hg.mozilla.org/mozilla-central/rev/56a690a7c1bc
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.