remove some unneeded includes in /mailnews

RESOLVED FIXED in Thunderbird 60.0

Status

enhancement
P5
trivial
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: aceman, Assigned: aceman)

Tracking

Trunk
Thunderbird 60.0

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: only check in when nothing else can be checked in and you want to invoke a new build on trunk)

Attachments

(4 attachments, 1 obsolete attachment)

There are some includes in the /mailnews c++ files (.h, .cpp, .mm) that seem unneeded, the code does not use any identifier from the included file.
I've made a quick script to identify where nsI* objects aren't used in the file, but nsI*.h file is included. Many of them are unneeded.
Cleaning up the needless includes may speed up compiling by a tiny bit ;)

I will attach several patches into this bug that cover the individual code directories under /mailnews.
There is no priority to check them in after review.
Jorg, you can use the patches for check-ins when needed, similar to bug 1399756.
Yep, slowly running out of white-space patches.
Comment on attachment 8927068 [details] [diff] [review]
1416033-addrbook.patch [checked in comment #14]

Sorry, please find another reviewer.
Attachment #8927068 - Flags: review?(jorgk) → review+
Attachment #8927068 - Flags: review+
Attachment #8927084 - Flags: review?(jorgk)
Attachment #8927086 - Flags: review?(jorgk)
Attachment #8927068 - Flags: review?(mkmelin+mozilla)
Attachment #8927084 - Flags: review?(mkmelin+mozilla)
Attachment #8927086 - Flags: review?(mkmelin+mozilla)
Attachment #8927068 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8927084 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8927086 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed
Whiteboard: only check in when nothing else can be checked in and you want to invoke a new build on trunk
Comment on attachment 8927068 [details] [diff] [review]
1416033-addrbook.patch [checked in comment #14]

Review of attachment 8927068 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/addrbook/src/nsAbLDAPDirectory.cpp
@@ +17,5 @@
>  #include "nsEnumeratorUtils.h"
>  #include "nsIAbLDAPAttributeMap.h"
>  #include "nsIAbMDBDirectory.h"
> +#include "nsIAbManager.h"
> +#include "nsIAddrDatabase.h"

Why add nsIAddrDatabase.h? That wasn't needed before. I wasn trying to compile without it, but somehow my build turned into a rebuild :-(
Comment on attachment 8927084 [details] [diff] [review]
1416033-db.patch

Bitrot here :-(
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/207bc898d11b
remove some unneeded includes in mailnews/intl. r=mkmelin
Keywords: checkin-needed
Attachment #8927086 - Attachment description: 1416033-intl.patch → 1416033-intl.patch [landed comment #8]
(In reply to Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?] from comment #6)
> Why add nsIAddrDatabase.h? That wasn't needed before.
Hmm, doesn't compile without it, I'm sure you can explain why that's needed now, or better, how that got included before.
Keywords: checkin-needed
Refreshed and while I was there, I removed nsAlgorithm.h and <algorithm>.
Attachment #8927084 - Attachment is obsolete: true
Attachment #8931831 - Flags: review+
Attachment #8931831 - Flags: feedback?(acelists)
(In reply to Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?] from comment #9)
> (In reply to Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?] from
> comment #6)
> > Why add nsIAddrDatabase.h? That wasn't needed before.
> Hmm, doesn't compile without it, I'm sure you can explain why that's needed
> now, or better, how that got included before.

I wouldn't add it if it didn't complain about it :)
I think I removed some include in a .h file which was also pulling in nsIAddrDatabase.h and that nsAbLDAPDirectory.cpp included. So when the parent doesn't have nsIAddrDatabase.h now, I added it to the child nsAbLDAPDirectory.cpp.
Comment on attachment 8931831 [details] [diff] [review]
1416033-db.patch (v2) [checked in comment #14]

Review of attachment 8931831 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the cleanup, if those aren't needed either.
Attachment #8931831 - Flags: feedback?(acelists) → feedback+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d95ef8a28c21
remove some unneeded includes in mailnews/addrbook. r=mkmelin
https://hg.mozilla.org/comm-central/rev/97cf0d5fff72
remove some unneeded includes in mailnews/db. r=mkmelin
Keywords: checkin-needed
Attachment #8927068 - Attachment description: 1416033-addrbook.patch → 1416033-addrbook.patch [checked in comment #14]
Attachment #8931831 - Attachment description: 1416033-db.patch (v2) → 1416033-db.patch (v2) [checked in comment #14]
Comment on attachment 8931888 [details] [diff] [review]
1416033-algorithm.patch [checked in comment #16]

Review of attachment 8931888 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.
Attachment #8931888 - Flags: review?(acelists) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/ede07b1d8a69
removed include of nsAlgorithm.h and <algorithm> from mailnews/ were not needed. r=aceman
Attachment #8931888 - Attachment description: 1416033-algorithm.patch → 1416033-algorithm.patch [checked in comment #16]
Keywords: leave-open
Target Milestone: --- → Thunderbird 60.0
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.