Closed
Bug 466732
Opened 16 years ago
Closed 16 years ago
Move from SIDump to log4moz usage
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b2
People
(Reporter: davida, Assigned: rain1)
References
()
Details
Attachments
(1 file, 1 obsolete file)
24.77 KB,
patch
|
rain1
:
review+
rain1
:
superreview+
|
Details | Diff | Splinter Review |
the URL above shows a bunch of SIDumps which should be log4moz debug statements.
Assignee | ||
Comment 1•16 years ago
|
||
Some points: - The basic idea used is the same as gloda, though at a smaller scale. - The idea is to eventually make nsWinSearch/SpotlightIntegration.js a module, so I've used this._log, etc. SearchIntegration is currently the "global scope" of the component. - I had to make some changes to the gloda code to avoid messages appearing twice, so CCing asuth (and also asking for review on changes). I simply moved the appender to the "gloda" scope instead of the "root" scope. - Unfortunately, this change exposed bug 467085, and the fix for that needs to be taken before this will work properly.
Assignee | ||
Updated•16 years ago
|
Attachment #350478 -
Flags: superreview?(bienvenu)
Attachment #350478 -
Flags: review?(bienvenu)
Assignee | ||
Updated•16 years ago
|
Attachment #350478 -
Flags: review?(bugmail)
Comment 3•16 years ago
|
||
Comment on attachment 350478 [details] [diff] [review] patch v1 this seems inconsistent about using "this._log" vs. "SearchIntegration._log" - is there any reason for that?
Assignee | ||
Comment 4•16 years ago
|
||
(In reply to comment #3) > (From update of attachment 350478 [details] [diff] [review]) > this seems inconsistent about using "this._log" vs. "SearchIntegration._log" - > is there any reason for that? That's because of scope rules -- if we're inside another object, then "_log" isn't a part of "this", and we have to use the explicit name. If we aren't, then we can use this._log as usual.
Comment 5•16 years ago
|
||
Comment on attachment 350478 [details] [diff] [review] patch v1 ok, thx. re the gloda changes, is the local var root needed after the other changes?
Attachment #350478 -
Flags: superreview?(bienvenu)
Attachment #350478 -
Flags: superreview+
Attachment #350478 -
Flags: review?(bienvenu)
Attachment #350478 -
Flags: review+
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #5) > (From update of attachment 350478 [details] [diff] [review]) > ok, thx. re the gloda changes, is the local var root needed after the other > changes? I don't think it is -- I'll change it to not use a var locally. This has bitrotted slightly due to bug 467087 -- I'll post an unbitrotted version with necessary modifications once asuth reviews it.
Updated•16 years ago
|
Attachment #350478 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 7•16 years ago
|
||
Continuing r+sr=bienvenu, r=asuth for gloda changes. I addressed bienvenu's comment, and also moved the prefs into all-thunderbird.js, as that's the more natural place for a mail/ only component.
Attachment #350478 -
Attachment is obsolete: true
Attachment #351696 -
Flags: superreview+
Attachment #351696 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 8•16 years ago
|
||
Comment on attachment 351696 [details] [diff] [review] addressed comments [Checkin: Comment 8] http://hg.mozilla.org/comm-central/rev/c59db8329e37
Attachment #351696 -
Attachment description: addressed comments → addressed comments
[Checkin: Comment 8]
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: --- → Thunderbird 3.0b2
You need to log in
before you can comment on or make changes to this bug.
Description
•