Closed
Bug 218825
Opened 21 years ago
Closed 21 years ago
Remove nsIFolder.idl (and nsMsgFolder.*)
Categories
(MailNews Core :: Backend, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: neil, Assigned: sspitzer)
Details
(Keywords: memory-footprint, perf)
Attachments
(6 files, 1 obsolete file)
72.43 KB,
patch
|
Bienvenu
:
review+
Henry.Jia
:
superreview+
|
Details | Diff | Splinter Review |
7.96 KB,
patch
|
Bienvenu
:
review+
Henry.Jia
:
superreview+
|
Details | Diff | Splinter Review |
30.36 KB,
text/plain
|
Details | |
1.38 KB,
patch
|
Bienvenu
:
review+
Henry.Jia
:
superreview+
|
Details | Diff | Splinter Review |
90.67 KB,
patch
|
Bienvenu
:
review+
Henry.Jia
:
superreview+
|
Details | Diff | Splinter Review |
3.60 KB,
patch
|
Bienvenu
:
review+
Henry.Jia
:
superreview+
|
Details | Diff | Splinter Review |
IMHO nsIFolder serves no useful purpose, it just takes up time and space.
Steps to remove nsIFolder:
1. Integrate nsMsgFolder into nsMsgDBFolder (nice, but not essential)
2. Convert all users of nsIFolder to nsIMsgFolder
3. Move nsIFolder definitions into nsIMsgFolder (+ nsMsgFolder.h if not 1.)
4. Remove nsIFolder from the build
Additionally we could review some other interfaces e.g. nsIFolderListener could
use nsIMsgFolder on all of its item parameters.
Reporter | ||
Comment 1•21 years ago
|
||
Reporter | ||
Updated•21 years ago
|
Attachment #131695 -
Flags: superreview?(scott)
Attachment #131695 -
Flags: review?(bienvenu)
Comment 2•21 years ago
|
||
this looks good, r=bienvenu with some nits,
+ *aParentMsgFolder = parent;
+ NS_IF_ADDREF(*aParentMsgFolder);
the assignment can be inside the NS_IF_ADDREF (which is apparently templatized
for this very reason)
same here:
+ *aRootMsgFolder = m_rootFolder;
+ NS_IF_ADDREF(*aRootMsgFolder);
here, please use the prevelant braces style
+ if (path.Length() == 1) {
+ NS_ADDREF(*aFolder = rootFolder);
+ return NS_OK;
+ }
so
if (path.Length() == 1)
{
}
Reporter | ||
Comment 3•21 years ago
|
||
As well as those three fixes I also fixed a second use of ) {, plus I deleted
an unused nsIFolder (rather than renaming it to nsIMsgFolder).
Reporter | ||
Updated•21 years ago
|
Attachment #131695 -
Attachment is obsolete: true
Reporter | ||
Comment 4•21 years ago
|
||
Comment on attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot
Transferring requests.
Attachment #131728 -
Flags: superreview?(scott)
Attachment #131728 -
Flags: review?(bienvenu)
Reporter | ||
Updated•21 years ago
|
Attachment #131695 -
Flags: superreview?(scott)
Attachment #131695 -
Flags: review?(bienvenu)
Updated•21 years ago
|
Attachment #131728 -
Flags: review?(bienvenu) → review+
Reporter | ||
Comment 5•21 years ago
|
||
Comment on attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot
Trying different reviewer.
Attachment #131728 -
Flags: superreview?(scott) → superreview?(dmose)
Comment on attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot
sr=Henry
Attachment #131728 -
Flags: superreview?(dmose) → superreview+
Reporter | ||
Comment 7•21 years ago
|
||
Reporter | ||
Updated•21 years ago
|
Attachment #135414 -
Flags: superreview?(Henry.Jia)
Attachment #135414 -
Flags: review?(bienvenu)
Updated•21 years ago
|
Attachment #135414 -
Flags: review?(bienvenu) → review+
Attachment #135414 -
Flags: superreview?(Henry.Jia) → superreview+
Reporter | ||
Comment 8•21 years ago
|
||
The next patch removes nsMsgFolder.cpp and moves all of its methods into
nsMsgDBFolder.cpp, except where they are already overridden. This bloats the
size of the patch, so what I have done here is to concatenate nsMsgDBFolder.cpp
and nsMsgFolder.cpp and rename the functions so you can see there isn't much
difference between the contatenation and the new nsMsgDBFolder.cpp
Reporter | ||
Comment 9•21 years ago
|
||
Don't know how I manged to leave these out :-[
Reporter | ||
Updated•21 years ago
|
Attachment #135496 -
Flags: superreview?(Henry.Jia)
Attachment #135496 -
Flags: review?(bienvenu)
Comment 10•21 years ago
|
||
Comment on attachment 135496 [details] [diff] [review]
Supplement to attachment 135414 [details] [diff] [review]
r/sr=bienvenu
Attachment #135496 -
Flags: review?(bienvenu) → review+
Reporter | ||
Comment 11•21 years ago
|
||
In answer to my rhetorical question, probably the same way that I managed to
leave out the "a" between the "n" and "g" in "manged" ;-)
Reporter | ||
Comment 12•21 years ago
|
||
As an alternative to reviewing the last two patches together, the previous
patch may be reviewed separately if the line NS_DECL_NSIFOLDER is temporarily
added between NS_DECL_NSIMSGFOLDER and NS_DECL_NSICOLLECTION.
Reporter | ||
Updated•21 years ago
|
Attachment #135499 -
Flags: superreview?(Henry.Jia)
Attachment #135499 -
Flags: review?(bienvenu)
Reporter | ||
Updated•21 years ago
|
Attachment #135507 -
Flags: superreview?(Henry.Jia)
Attachment #135507 -
Flags: review?(bienvenu)
Updated•21 years ago
|
Attachment #135507 -
Flags: review?(bienvenu) → review+
Comment 13•21 years ago
|
||
Comment on attachment 135499 [details] [diff] [review]
Delete nsMsgFolder.* and move to nsMsgDBFolder.*
there's lot of stuff that could be cleaned up (nit-type stuff in the existing
code) but it's all code that was simply moved. Let's just make sure this works
and then we can clean up other stuff later.
Attachment #135499 -
Flags: review?(bienvenu) → review+
Attachment #135496 -
Flags: superreview?(Henry.Jia) → superreview+
Attachment #135499 -
Flags: superreview?(Henry.Jia) → superreview+
Attachment #135507 -
Flags: superreview?(Henry.Jia) → superreview+
Reporter | ||
Comment 14•21 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 15•21 years ago
|
||
I filled bug 227175 (leak in a collation).
Is it possible that kCollationKeyGenerator is not freed?
Updated•20 years ago
|
Product: MailNews → Core
Updated•17 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•