Closed
Bug 820570
Opened 11 years ago
Closed 11 years ago
Move DebugOnly into DebugOnly.h
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(1 file)
74.84 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
I would be very unsurprised if this doesn't rely on bootlegged includes somewhere, but it gets the job done to the point of adding the new header and removing the definitions from Util.h. Most of the places where I remove a Util.h include, I observed nothing else mozilla:: being used. I'm not sure whether to be surprised or unsurprised that most people wanting Util.h use it just for DebugOnly. Probably unsurprised, given the rest of Util.h is rather less commonly useful.
Attachment #691059 -
Flags: review?(Ms2ger)
Comment 1•11 years ago
|
||
Comment on attachment 691059 [details] [diff] [review] Patch Review of attachment 691059 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/html/HTMLTableAccessible.cpp @@ +8,1 @@ > #include "HTMLTableAccessible.h" In general, I think Foo.h should be included first in Foo.cpp, to make sure it compiles on its own, but I won't ask you to do that :) ::: dom/bindings/BindingUtils.cpp @@ +9,2 @@ > #include <algorithm> > #include <stdarg.h> But I think we're pretty consistent about std headers first, so please do put DebugOnly below these. ::: dom/devicestorage/nsDeviceStorage.cpp @@ -12,5 @@ > #include "mozilla/dom/devicestorage/PDeviceStorageRequestChild.h" > #include "mozilla/dom/ipc/Blob.h" > #include "mozilla/dom/PBrowserChild.h" > #include "mozilla/dom/PContentPermissionRequestChild.h" > -#include "mozilla/Util.h" // DebugOnly IMO, we should just sort lexicographically, not split on MFBT/non-MFBT. ::: js/src/frontend/TokenStream.h @@ +12,3 @@ > /* > * JS lexical scanner interface. > */ Not after this comment?
Attachment #691059 -
Flags: review?(Ms2ger) → review+
Assignee | ||
Comment 2•11 years ago
|
||
I looked back at every change in the patch, even made many of the <>-first changes requested, and it's definitely not the case that we're consistent about <>-first. :-) Nor are we really consistent on any particular point. That goes for JS, too -- we're better about having some sort of ideal, but we only kind of stick to it. Anyway, I did a certain amount of fixup here, and as I said looked at every file, but I probably missed some stuff and/or didn't change it all quite consistently. Still, this is an improvement. https://hg.mozilla.org/integration/mozilla-inbound/rev/18bc32f799d1
Target Milestone: --- → mozilla20
Assignee | ||
Comment 3•11 years ago
|
||
Oh, I think someone's going to need to do a little tweaking to comm-central when this lands: http://mxr.mozilla.org/comm-central/search?string=DebugOnly&find=mailnews&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central That comes down to just fixing mailnews/addrbook/src/nsAbView.cpp to include DebugOnly.h, if that comment's correct by doing away with the Util.h include.
Comment 4•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/18bc32f799d1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•