Closed Bug 466732 Opened 13 years ago Closed 13 years ago

Move from SIDump to log4moz usage

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: davida, Assigned: rain1)

References

()

Details

Attachments

(1 file, 1 obsolete file)

the URL above shows a bunch of SIDumps which should be log4moz debug statements.
Blocks: 430614
Depends on: 467085
Depends on: 467087
Attached patch patch v1 (obsolete) — Splinter Review
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.
Attachment #350478 - Flags: superreview?(bienvenu)
Attachment #350478 - Flags: review?(bienvenu)
Attachment #350478 - Flags: review?(bugmail)
Duplicate of this bug: 465951
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?
(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 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+
(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.
Attachment #350478 - Flags: review?(bugmail) → review+
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+
Keywords: checkin-needed
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]
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
You need to log in before you can comment on or make changes to this bug.