Closed Bug 820570 Opened 11 years ago Closed 11 years ago

Move DebugOnly into DebugOnly.h

Categories

(Core :: MFBT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: Waldo, Assigned: Waldo)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter 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 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+
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
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.
https://hg.mozilla.org/mozilla-central/rev/18bc32f799d1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 822263
You need to log in before you can comment on or make changes to this bug.