1025976 - Remove unused folder flag GotNew

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Kent James (:rkent)]

As far as I can tell, the folder flag GotNew is set and cleared in a few places in mailnews, but never actually used for anything. Rather than try to fix it or test it in bug 1025113, I'd like to just remove it. There are already too many other indicators of new messages floating around.

Comment 1

[Bogdan Stefan]

Attachment: Bug 1025976 patch

I think I removed the flag completely.
Ran the unit tests successfully.

This is my first Thunderbird bug, is everything ok(the patch submission)?

Comment 2

[Magnus Melin [:mkmelin]]

Didn't look in detail yet, but one thing you should do is rev the uuid for the nsMsgFolderFlags.idl as it's changed.
(Generate a new uuid, on linux you can use the uuidgen command)

Then see http://developer.mozilla.org/en/docs/Getting_your_patch_in_the_tree#Getting_the_patch_reviewed

Comment 3

[Bogdan Stefan]

Attachment: 1025976 patch v2

How's this?

So every time a idl file is modified, i need to change its UUID?

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 8728050 [details] [diff] [review]
1025976 patch v2

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

Yes, this looks good. r=mkmelin
Thx Bodgan!

Comment 5

[:aceman]

Isn't the style in that file to NOT remove unused flags, but rename them to UnusedX ?

Comment 6

[Kent James (:rkent)]

(In reply to :aceman from comment #5)
> Isn't the style in that file to NOT remove unused flags, but rename them to
> UnusedX ?

Yes that would be preferable.

Comment 7

[:aceman]

Bogdan, please update the patch to just rename the GotNew to Unused7 (or what number comes next) and update the comment above it to say what that constant was used for in the past. See Unused4 above.
It is so that we do not erroneosly reuse that flag value for other purposes. Existing folders may still contain that value it we just need to ignore it and not suddenly make it mean something else.

Comment 8

[Bogdan Stefan]

Attachment: 1025976 patch

Here it is. Sorry for the mistake.

Comment 9

[:aceman]

Comment on attachment 8735100 [details] [diff] [review]
1025976 patch

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

Yes, thanks. I haven't run the tests but only checked the change in nsMsgFolderFlags.idl. I assume Magnus did review it fully for the first r+.

::: mailnews/base/public/nsMsgFolderFlags.idl
@@ +76,5 @@
>    /// This used to be used for virtual newsgroups
>    const nsMsgFolderFlagType Unused1         = 0x00008000;
>    /// Used to be for categories
>    const nsMsgFolderFlagType Unused4         = 0x00010000;
> +  /// Used to be for new msgs in a folder    

Please remove the trailing whitespace you add here.

Comment 10

[Bogdan Stefan]

Attachment: 1025976 patch

Ugh, Atom did it.

Well I've run the tests the first time and they are fine.
I guess Magnus did it too.

Comment 11

[:aceman]

Comment on attachment 8735111 [details] [diff] [review]
1025976 patch

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

Thanks.

Comment 12

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/d9df380b42b0786499f96ace95c64a53e5444389
Bug 1025976 - Remove GotNew folder flag. r=aceman