Closed
Bug 577914
Opened 14 years ago
Closed 14 years ago
mark DEBUG only variables as ifdef DEBUG in layout
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla5
People
(Reporter: timeless, Assigned: timeless)
References
Details
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
dbaron
:
review+
dbaron
:
approval2.0-
|
Details | Diff | Splinter Review |
this is part of a crusade to get rid of compilation warnings
|
||
Comment 2•14 years ago
|
||
This makes the code pretty unreadable; I don't think this is worth it to get rid of compiler warnings.
|
|
||
Comment 3•14 years ago
|
||
Comment on attachment 456741 [details] [diff] [review]
patch
see previous comment
Attachment #456741 -
Flags: review?(dbaron) → review-
|
|
||
Comment 4•14 years ago
|
||
An alternative that might be a *little* more readable would be a general NS_IF_DEBUG() macro that makes its argument appear ifdef debug, and disappear otherwise, so you'd write:
NS_IF_DEBUG(nsIContent* propagatedScrollFrom =) PropagateScrollToViewport();
It still makes our code a lot less approachable to outsiders, though, so I'm not especially happy about that, either.
Attachment #456741 -
Attachment is obsolete: true
Attachment #463501 -
Flags: review?(dbaron)
Summary: use NS_DEBUG_ASSIGN for layout → mark DEBUG only variables as ifdef DEBUG in layout
|
|
||
Updated•14 years ago
|
Attachment #463501 -
Flags: review?(dbaron) → review+
Attachment #463501 -
Flags: approval2.0?
|
|
||
Comment 6•14 years ago
|
||
Comment on attachment 463501 [details] [diff] [review]
using #ifdef DEBUG
This should land after we branch for Gecko 2.0 / Firefox 4.
Attachment #463501 -
Flags: approval2.0? → approval2.0-
|
||
Comment 7•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
|
||
Comment 8•14 years ago
|
||
Did we really want to do this (and the other similar patches for other modules that just landed as well) at this point, in view of the work-in-progress in bug 577899?
|
|
||
Comment 9•14 years ago
|
||
(In reply to comment #8)
> Did we really want to do this (and the other similar patches for other modules
> that just landed as well) at this point, in view of the work-in-progress in bug
> 577899?
These patches were reviewed, that's why I landed them. I personally didn't know about bug 577899 before this, but I'm not sure what we want to do now (back all of those patches out, switch them over to NS_DEBUG_ONLY, etc).
You need to log in
before you can comment on or make changes to this bug.
Description
•