Closed Bug 467085 Opened 14 years ago Closed 14 years ago

log4moz updateParents messed up, doesn't support multilevel parenting properly

Categories

(Cloud Services :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rain1, Assigned: rain1)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch fix, without unit tests (obsolete) — Splinter Review
The parent detection code is overly optimistic in trying to find a parent, and finds itself in this._loggers, thus thinking that it has no parent closer to itself than root.

I've tested this manually and it works, and I'll also attach a unit test once I have the xulrunner SDK set up.
Attachment #350475 - Flags: review?(thunder)
Summary: log4moz updateParents messed up, doesn't support more than one level below root → log4moz updateParents messed up, doesn't support multilevel parenting properly
Blocks: 467087
One more bug fixed -- that really should hav been the other way round ;)

Unit tests now added, and they pass only with the patch applied.

thunder: are you the right person to ask for review?
Attachment #350475 - Attachment is obsolete: true
Attachment #350979 - Flags: review?(thunder)
Attachment #350475 - Flags: review?(thunder)
I am, and thanks!  I will review this today.
Looks good on the surface, though I will give it a more careful read later today.

One nit: use the name getter for the loggers, instead of the _name 'private' property.
Comment on attachment 350979 [details] [diff] [review]
Updated patch to fix one more bug, and now with shiny new unit tests

Great job on both bugs, and the tests.  Thanks!

Do you have commit access?
Attachment #350979 - Flags: review?(thunder) → review+
Continuing r=thunder

Thanks for the review! No, I don't have commit access.
Attachment #350979 - Attachment is obsolete: true
Attachment #351381 - Flags: review+
Keywords: checkin-needed
Attached file Bundle to checkin
pushed
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Component: Weave → General
Product: Mozilla Labs → Weave
Target Milestone: -- → ---
QA Contact: weave → general
You need to log in before you can comment on or make changes to this bug.