Closed Bug 703068 Opened 13 years ago Closed 11 years ago

Use MOZ_OVERRIDE in mailnews/

Categories

(MailNews Core :: Backend, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 25.0

People

(Reporter: rain1, Assigned: Six)

References

()

Details

Attachments

(9 files, 35 obsolete files)

5.65 KB, patch
standard8
: review+
Details | Diff | Splinter Review
20.74 KB, patch
standard8
: review+
Details | Diff | Splinter Review
50.97 KB, patch
mconley
: review+
Details | Diff | Splinter Review
60.62 KB, patch
standard8
: review+
Details | Diff | Splinter Review
46.56 KB, patch
standard8
: review+
Details | Diff | Splinter Review
34.22 KB, patch
standard8
: review+
Details | Diff | Splinter Review
12.21 KB, patch
standard8
: review+
Details | Diff | Splinter Review
10.84 KB, patch
Six
: review+
Details | Diff | Splinter Review
7.19 KB, patch
Six
: review+
Details | Diff | Splinter Review
This is going to help us catch bugs where the signature of an override doesn't match and so doesn't get called.

bienvenu requested that this be done on top of bug 402392.
Depends on: 402392
Sid ?
Attachment #747590 - Flags: review?(sid.bugzilla)
Comment on attachment 747590 [details] [diff] [review]
Add ~200 MOZ_OVERRIDE in mailnews/

This patch is way too big for me to have the time to review. Please ask someone else, preferably individual module owners.
Attachment #747590 - Flags: review?(sid.bugzilla)
Assignee: nobody → six.dsn
Status: NEW → ASSIGNED
Comment on attachment 747590 [details] [diff] [review]
Add ~200 MOZ_OVERRIDE in mailnews/

Adding mconley for the ab part

Standard8 -> for LDAP

We should also have
Attachment #747590 - Flags: review?(mconley)
Attachment #747590 - Flags: feedback?(mbanner)
A new patch will be available soon,
i have improved the script that generates it, cf Bug 870516
there must be some missing overrides 
that should be fixed in the next pass
(In reply to Arnaud Sourioux [:Six] from comment #5)
> A new patch will be available soon,
> i have improved the script that generates it, cf Bug 870516
> there must be some missing overrides 
> that should be fixed in the next pass

Good can you split the patches by functional area (eg AB, Imap etc ...) so we can request reviews to proper peers (eg see https://wiki.mozilla.org/Modules/MailNews_Core) ?
Yes i guess it's the only way to make it right :)
I can provide one patch by mailnews main subfolder which looks kind of what you expect
So next patch will be done this way
thanks for the advice
Comment on attachment 747590 [details] [diff] [review]
Add ~200 MOZ_OVERRIDE in mailnews/

Cancelling review and feedback request as new patches are coming up.
Attachment #747590 - Flags: review?(mconley)
Attachment #747590 - Flags: feedback?(mbanner)
Attachment #749387 - Flags: review?(mconley)
Attachment #747590 - Attachment is obsolete: true
Attachment #749388 - Flags: review? → review?(mozilla)
Attachment #749391 - Flags: review? → review?(mozilla)
Attachment #749392 - Flags: review?(mozilla)
Attachment #749393 - Flags: review?(mozilla)
Attachment #749394 - Flags: review?(mbanner)
Attachment #749387 - Attachment description: Bug 703068: Annotate 22 methods with MOZ_OVERRIDE in mailnews/addrbook → Annotate 22 methods with MOZ_OVERRIDE in mailnews/addrbook
Attachment #749388 - Attachment description: Bug 703068: Annotate 101 methods with MOZ_OVERRIDE in mailnews/base → Annotate 101 methods with MOZ_OVERRIDE in mailnews/base
Attachment #749395 - Flags: review?(mbanner)
Attachment #749396 - Flags: review? → review?(mozilla)
Attachment #749398 - Flags: review?(Pidgeot18)
This is all the MOZ_OVERRIDE added to mailnews/ splitted by module and assigned for review to each module owner.
Edit them if some are wrong.
Adding Irving in case he has some spare cycle for review ?
Attachment #749387 - Attachment is obsolete: true
Attachment #749387 - Flags: review?(mconley)
Attachment #752903 - Flags: review?(mconley)
Attachment #749388 - Attachment is obsolete: true
Attachment #749388 - Flags: review?(mozilla)
Attachment #752905 - Flags: review?(mozilla)
Attachment #749391 - Attachment is obsolete: true
Attachment #749391 - Flags: review?(mozilla)
Attachment #752907 - Flags: review?(mozilla)
Attachment #749392 - Attachment is obsolete: true
Attachment #749392 - Flags: review?(mozilla)
Attachment #752908 - Flags: review?(mozilla)
Attachment #749393 - Attachment is obsolete: true
Attachment #749393 - Flags: review?(mozilla)
Attachment #752911 - Flags: review?(mozilla)
Attachment #749394 - Attachment is obsolete: true
Attachment #749394 - Flags: review?(mbanner)
Attachment #752912 - Flags: review?(mbanner)
Attachment #749395 - Attachment is obsolete: true
Attachment #749395 - Flags: review?(mbanner)
Attachment #752913 - Flags: review?(mbanner)
Attachment #749396 - Attachment is obsolete: true
Attachment #749396 - Flags: review?(mozilla)
Attachment #752915 - Flags: review?(mozilla)
Attachment #749398 - Attachment is obsolete: true
Attachment #749398 - Flags: review?(Pidgeot18)
Attachment #752916 - Flags: review?(Pidgeot18)
Comment on attachment 752903 [details] [diff] [review]
Annotate 51 methods with MOZ_OVERRIDE in mailnews/addrbook

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

Assuming http://whereswalden.com/2011/11/16/introducing-moz_override-to-annotate-virtual-functions-which-override-base-class-virtual-functions/ is an accurate description of what MOZ_OVERRIDE is supposed to do, this all looks sane to me.
Attachment #752903 - Flags: review?(mconley) → review+
Hi,
thanks for your review, i'm working on it since one year, 
the script that generates it is safe, 
the review is mainly to see if there are some miss.
There might be a new version in few days as we (with dholbert) improved little things that weren't caught as override. it may not affect your module.
Comment on attachment 752916 [details] [diff] [review]
Annotate 5 methods with MOZ_OVERRIDE in mailnews/news

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

::: mailnews/news/src/nsNewsDownloader.h
@@ +69,5 @@
>    DownloadNewsArticlesToOfflineStore(nsIMsgWindow *window, nsIMsgDatabase *db, nsIUrlListener *listener);
>    virtual ~DownloadNewsArticlesToOfflineStore();
>  
> +  NS_IMETHOD OnStartRunningUrl(nsIURI* url) MOZ_OVERRIDE;
> +  NS_IMETHOD OnStopRunningUrl(nsIURI* url, nsresult exitCode) MOZ_OVERRIDE;

nits: change nsIURI* url to nsIURI aUrl while you're changing these lines, and exitCode to aExitCode

@@ +74,2 @@
>  protected:
> +  virtual int32_t  StartDownload() MOZ_OVERRIDE;

Nit: lose the extra space here.
Attachment #752916 - Flags: review?(Pidgeot18) → review+
Ok i will change this by hand in the next patch,this is done by a script so i never change the content. thanks for the review
final version.
Attachment #752903 - Attachment is obsolete: true
Attachment #754382 - Flags: review?(mconley)
final version.
Attachment #752905 - Attachment is obsolete: true
Attachment #752905 - Flags: review?(mozilla)
Attachment #754383 - Flags: review?(mozilla)
final version.
Attachment #752907 - Attachment is obsolete: true
Attachment #752907 - Flags: review?(mozilla)
Attachment #754384 - Flags: review?
Attachment #752908 - Attachment is obsolete: true
Attachment #752908 - Flags: review?(mozilla)
Attachment #754385 - Flags: review?(mozilla)
Attachment #752911 - Attachment is obsolete: true
Attachment #752911 - Flags: review?(mozilla)
Attachment #754386 - Flags: review?(mozilla)
final version.
Attachment #752912 - Attachment is obsolete: true
Attachment #752912 - Flags: review?(mbanner)
Attachment #754387 - Flags: review?(mbanner)
final version.
Attachment #752913 - Attachment is obsolete: true
Attachment #752913 - Flags: review?(mbanner)
Attachment #754388 - Flags: review?(mbanner)
final version.
Attachment #752915 - Attachment is obsolete: true
Attachment #752915 - Flags: review?(mozilla)
Attachment #754389 - Flags: review?(mozilla)
final version.
Attachment #752916 - Attachment is obsolete: true
Attachment #754390 - Flags: review?(Pidgeot18)
Attachment #754384 - Flags: review? → review?(mozilla)
final version + 3 manual modifications asked in the last review.
Attachment #754390 - Attachment is obsolete: true
Attachment #754390 - Flags: review?(Pidgeot18)
Attachment #754391 - Flags: review?(Pidgeot18)
Attachment #754382 - Attachment is obsolete: true
Attachment #754382 - Flags: review?(mconley)
Attachment #755330 - Flags: review?(mconley)
Attachment #754383 - Attachment is obsolete: true
Attachment #754383 - Flags: review?(mozilla)
Attachment #755332 - Flags: review?(mozilla)
Attachment #754385 - Attachment is obsolete: true
Attachment #754385 - Flags: review?(mozilla)
Attachment #755333 - Flags: review?(mozilla)
Attachment #754386 - Attachment is obsolete: true
Attachment #754386 - Flags: review?(mozilla)
Attachment #755334 - Flags: review?(mozilla)
Attachment #754388 - Attachment is obsolete: true
Attachment #754388 - Flags: review?(mbanner)
Attachment #755336 - Flags: review?(mbanner)
Attachment #754391 - Attachment is obsolete: true
Attachment #754391 - Flags: review?(Pidgeot18)
Attachment #755339 - Flags: review?(Pidgeot18)
Comment on attachment 755339 [details] [diff] [review]
Annotate 73 methods with MOZ_OVERRIDE in mailnews/news + manual modifications

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

It's fine, but a lot of associated cleanup should be done while you're touching these lines.

::: mailnews/news/src/nsNNTPNewsgroupPost.h
@@ +84,1 @@
>      

Nit: clean up this trailing whitespace.

::: mailnews/news/src/nsNNTPProtocol.h
@@ +140,5 @@
>                   nsIMsgWindow *aMsgWindow);
>    virtual ~nsNNTPProtocol();
>  
>    // stop binding is a "notification" informing us that the stream associated with aURL is going away.
> +  NS_IMETHOD OnStopRequest(nsIRequest *request, nsISupports * aCtxt, nsresult aStatus) MOZ_OVERRIDE;

Nit: 80-char line width violations here...

@@ +146,5 @@
>    char * m_ProxyServer;    /* proxy server hostname */
>  
> +  NS_IMETHOD Cancel(nsresult status) MOZ_OVERRIDE;  // handle stop button
> +  NS_IMETHOD GetContentType(nsACString &aContentType) MOZ_OVERRIDE;
> +  NS_IMETHOD AsyncOpen(nsIStreamListener *listener, nsISupports *ctxt) MOZ_OVERRIDE;

...here...

@@ +172,5 @@
>     * the code will not read the input stream at all. Therefore, it is strongly
>     * advised to suspend the request before using this state.
>     */
>    virtual nsresult ProcessProtocolState(nsIURI * url, nsIInputStream * inputStream,
> +                                        uint64_t sourceOffset, uint32_t length) MOZ_OVERRIDE;

...here...

@@ +178,4 @@
>  
>    // we have our own implementation of SendData which writes to the nntp log
>    // and then calls the base class to transmit the data
> +  nsresult SendData(const char * dataBuffer, bool aSuppressLogging = false) MOZ_OVERRIDE;

...and here.

::: mailnews/news/src/nsNewsDownloader.h
@@ +74,2 @@
>  protected:
> +  virtual int32_t  StartDownload() MOZ_OVERRIDE;

Nit: lose the extra space.

::: mailnews/news/src/nsNewsFolder.h
@@ +40,2 @@
>  
> +  NS_IMETHOD CreateSubfolder(const nsAString& folderName,nsIMsgWindow *msgWindow) MOZ_OVERRIDE;

More 80-char violations here..

@@ +42,3 @@
>  
> +  NS_IMETHOD Delete () MOZ_OVERRIDE;
> +  NS_IMETHOD Rename (const nsAString& newName, nsIMsgWindow *msgWindow) MOZ_OVERRIDE;

...here...

@@ +49,3 @@
>  
>    NS_IMETHOD GetExpungedBytesCount(uint32_t *count);
> +  NS_IMETHOD GetDeletable (bool *deletable) MOZ_OVERRIDE;

(Extra space here)

@@ +56,2 @@
>  
> +  NS_IMETHOD GetDBFolderInfoAndDB(nsIDBFolderInfo **folderInfo, nsIMsgDatabase **db) MOZ_OVERRIDE;

...here...

@@ +58,5 @@
>  
>    NS_IMETHOD DeleteMessages(nsIArray *messages,
>                        nsIMsgWindow *msgWindow, bool deleteStorage, bool isMove,
> +                      nsIMsgCopyServiceListener* listener, bool allowUndo) MOZ_OVERRIDE;
> +  NS_IMETHOD GetNewMessages(nsIMsgWindow *aWindow, nsIUrlListener *aListener) MOZ_OVERRIDE;

...here as well...
(Also, fix the bad indentation for the DeleteMessages continuation?)

@@ +71,4 @@
>  
> +  NS_IMETHOD DownloadMessagesForOffline(nsIArray *messages, nsIMsgWindow *window) MOZ_OVERRIDE;
> +  NS_IMETHOD Compact(nsIUrlListener *aListener, nsIMsgWindow *aMsgWindow) MOZ_OVERRIDE;
> +  NS_IMETHOD DownloadAllForOffline(nsIUrlListener *listener, nsIMsgWindow *msgWindow) MOZ_OVERRIDE;

...all three of these...

@@ +79,3 @@
>  
> +  NS_IMETHOD GetFilterList(nsIMsgWindow *aMsgWindow, nsIMsgFilterList **aFilterList) MOZ_OVERRIDE;
> +  NS_IMETHOD GetEditableFilterList(nsIMsgWindow *aMsgWindow, nsIMsgFilterList **aFilterList) MOZ_OVERRIDE;

...both of these...

@@ +89,5 @@
>    nsresult ParseFolder(nsIFile *path);
>    nsresult CreateSubFolders(nsIFile *path);
>    nsresult AddDirectorySeparator(nsIFile *path);
> +  nsresult GetDatabase() MOZ_OVERRIDE;
> +  virtual nsresult CreateChildFromURI(const nsCString &uri, nsIMsgFolder **folder) MOZ_OVERRIDE;

...here...

@@ +98,5 @@
>    nsresult ForgetLine(void);
>    nsresult GetNewsMessages(nsIMsgWindow *aMsgWindow, bool getOld, nsIUrlListener *aListener);
>  
>    int32_t HandleNewsrcLine(const char * line, uint32_t line_size);
> +  virtual void GetIncomingServerType(nsCString& serverType) MOZ_OVERRIDE { serverType.AssignLiteral("nntp");}

...and here.

::: mailnews/news/src/nsNntpIncomingServer.h
@@ +66,5 @@
> +    NS_IMETHOD GetCanSearchMessages(bool *canSearchMessages) MOZ_OVERRIDE;
> +    NS_IMETHOD GetOfflineSupportLevel(int32_t *aSupportLevel) MOZ_OVERRIDE;
> +    NS_IMETHOD GetDefaultCopiesAndFoldersPrefsToServer(bool *aCopiesAndFoldersOnServer) MOZ_OVERRIDE;
> +    NS_IMETHOD GetCanCreateFoldersOnServer(bool *aCanCreateFoldersOnServer) MOZ_OVERRIDE;
> +    NS_IMETHOD GetCanFileMessagesOnServer(bool *aCanFileMessagesOnServer) MOZ_OVERRIDE;

Some more 80-char violations in this file.

@@ +77,4 @@
>  
>  protected:
>     virtual nsresult CreateRootFolderFromUri(const nsCString &serverUri,
> +                                            nsIMsgFolder **rootFolder) MOZ_OVERRIDE;

Again.

@@ +90,5 @@
>       * Downloads the newsgroup headers.
>       */
>      nsresult DownloadMail(nsIMsgWindow *aMsgWindow);
>  
> +    NS_IMETHOD GetServerRequiresPasswordForBiff(bool *aServerRequiresPasswordForBiff) MOZ_OVERRIDE;

and again.

Actually, for the last one, just changing the name to "retval" would fix the issue much better.
Attachment #755339 - Flags: review?(Pidgeot18) → review+
Comment on attachment 754387 [details] [diff] [review]
Annotate 49 methods with MOZ_OVERRIDE in mailnews/import

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

Looks good. I would normally ask for whitespace cleanup, however as the import code is so badly formatted anyway, it'd end up changing whole files which would be a separate bug anyway. So lets take it as it is.
Attachment #754387 - Flags: review?(mbanner) → review+
Comment on attachment 755330 [details] [diff] [review]
Annotate 196 methods with MOZ_OVERRIDE in mailnews/addrbook

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

Just one nit, but other than that, this looks fine to me.

::: mailnews/addrbook/src/nsAbBSDirectory.h
@@ +22,5 @@
>  	virtual ~nsAbBSDirectory();
>  
>  	// nsIAbDirectory methods
> +  NS_IMETHOD Init(const char *aURI) MOZ_OVERRIDE;
> +	NS_IMETHOD GetChildNodes(nsISimpleEnumerator* *result) MOZ_OVERRIDE;

While you're here - please fix the indentation on this line.
Attachment #755330 - Flags: review?(mconley) → review+
It tried to apply all your comments, it should be ok

::: mailnews/news/src/nsNntpIncomingServer.h
@@ +66,5 @@
> +    NS_IMETHOD GetCanSearchMessages(bool *canSearchMessages) MOZ_OVERRIDE;
> +    NS_IMETHOD GetOfflineSupportLevel(int32_t *aSupportLevel) MOZ_OVERRIDE;
> +    NS_IMETHOD GetDefaultCopiesAndFoldersPrefsToServer(bool *aCopiesAndFoldersOnServer) MOZ_OVERRIDE;
> +    NS_IMETHOD GetCanCreateFoldersOnServer(bool *aCanCreateFoldersOnServer) MOZ_OVERRIDE;
> +    NS_IMETHOD GetCanFileMessagesOnServer(bool *aCanFileMessagesOnServer) MOZ_OVERRIDE;

This is looking ugly in the patch but not other choice except change the parameter name
Attachment #755339 - Attachment is obsolete: true
Attachment #764033 - Flags: review?(Pidgeot18)
The same patch with only the indentation fixed.
Attachment #755330 - Attachment is obsolete: true
Attachment #764034 - Flags: review?(mconley)
Attachment #754384 - Flags: review?(mozilla) → review+
Comment on attachment 754389 [details] [diff] [review]
Annotate 32 methods with MOZ_OVERRIDE in mailnews/mime

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

Looks good, r=Standard8 with the comments fixed.

::: mailnews/mime/emitters/nsMimeHtmlEmitter.h
@@ +42,4 @@
>  
> +    virtual nsresult            WriteHeaderFieldHTMLPrefix(const nsACString &name) MOZ_OVERRIDE;
> +    virtual nsresult            WriteHeaderFieldHTML(const char *field, const char *value) MOZ_OVERRIDE;
> +    virtual nsresult            WriteHeaderFieldHTMLPostfix() MOZ_OVERRIDE;

It'd be nice if you could remove the unnecessary blank whitespace from these lines and cut down the length a bit.

::: mailnews/mime/emitters/nsMimeRawEmitter.h
@@ +18,5 @@
>  public: 
>      nsMimeRawEmitter ();
>      virtual       ~nsMimeRawEmitter (void);
>  
> +    NS_IMETHOD    WriteBody(const nsACString &buf, uint32_t *amountWritten) MOZ_OVERRIDE;

nit: could you wrap this line a bit, maybe in between the two parameters.

::: mailnews/mime/src/nsMimeObjectClassAccess.h
@@ +45,2 @@
>  
> +  NS_IMETHOD    MimeCreate(char *content_type, void * hdrs, void * opts, void**ptr) MOZ_OVERRIDE;

nit: can you wrap this line a bit?
Attachment #754389 - Flags: review?(mozilla) → review+
Comment on attachment 755332 [details] [diff] [review]
Annotate 161 methods with MOZ_OVERRIDE in mailnews/base

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

Looks good, I've commented on a few lines that need wrapping, but it'd be nice if you could tidy up some of the other long ones that this patch affects as well.

::: mailnews/base/src/nsMsgAccountManagerDS.h
@@ +45,5 @@
>      NS_DECL_NSIINCOMINGSERVERLISTENER
>      NS_DECL_NSIOBSERVER
>    // RDF datasource methods
>    
> +  /* nsIRDFNode GetTarget (in nsIRDFResource aSource, in nsIRDFResource property, in boolean aTruthValue) MOZ_OVERRIDE; */

nit: please just remove this comment, there's no actual use it in anyway.

::: mailnews/base/src/nsMsgFolderDataSource.h
@@ +69,2 @@
>  
> +  NS_IMETHOD HasArcOut(nsIRDFResource *aSource, nsIRDFResource *aArc, bool *result) MOZ_OVERRIDE;

nit: could do with a little wrapping.

@@ +79,3 @@
>  
>    NS_IMETHOD GetAllCmds(nsIRDFResource* source,
> +                            nsISimpleEnumerator/*<nsIRDFResource>*/** commands) MOZ_OVERRIDE;

nit: wrapping

@@ +294,2 @@
>  protected:
> +  virtual nsresult GetFolderDisplayName(nsIMsgFolder *folder, nsString& folderName) MOZ_OVERRIDE;

nit: wrapping

@@ +296,4 @@
>    virtual void EnsureFolders();
>    virtual bool WantsThisFolder(nsIMsgFolder *folder);
>            bool ResourceIsOurRoot(nsIRDFResource *resource);
> +  virtual nsresult OnItemAddedOrRemoved(nsIMsgFolder *parentItem, nsISupports *item, bool added) MOZ_OVERRIDE;

nit: wrapping

::: mailnews/base/src/nsMsgGroupThread.h
@@ +71,5 @@
>  protected:
> +  virtual void InsertMsgHdrAt(nsMsgViewIndex index, nsIMsgDBHdr *hdr) MOZ_OVERRIDE;
> +  virtual void SetMsgHdrAt(nsMsgViewIndex index, nsIMsgDBHdr *hdr) MOZ_OVERRIDE;
> +  virtual nsMsgViewIndex FindMsgHdr(nsIMsgDBHdr *hdr) MOZ_OVERRIDE;
> +  virtual nsMsgViewIndex AddMsgHdrInDateOrder(nsIMsgDBHdr *child, nsMsgDBView *view) MOZ_OVERRIDE;

nit: wrapping
Attachment #755332 - Flags: review?(mozilla) → review+
Comment on attachment 755333 [details] [diff] [review]
Annotate 40 methods with MOZ_OVERRIDE in mailnews/db

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

::: mailnews/db/msgdb/public/nsMailDatabase.h
@@ +22,5 @@
>  public:
>    nsMailDatabase();
>    virtual ~nsMailDatabase();
> +  NS_IMETHOD  ForceClosed() MOZ_OVERRIDE;
> +  NS_IMETHOD DeleteMessages(uint32_t aNumKeys, nsMsgKey* nsMsgKeys, nsIDBChangeListener *instigator) MOZ_OVERRIDE;

nit: line length

@@ +35,2 @@
>    
> +  NS_IMETHOD  GetOfflineOpForKey(nsMsgKey opKey, bool create, nsIMsgOfflineImapOperation **op) MOZ_OVERRIDE;

nit: line length
Attachment #755333 - Flags: review?(mozilla) → review+
Comment on attachment 755334 [details] [diff] [review]
Annotate 163 methods with MOZ_OVERRIDE in mailnews/imap

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

::: mailnews/imap/src/nsIMAPBodyShell.h
@@ +134,3 @@
>      // Generates an HTML representation of this part.  Returns content length generated, -1 if failed.
> +    virtual int32_t Generate(nsIMAPBodyShell *aShell, bool stream, bool prefetch) MOZ_OVERRIDE;
> +	virtual nsIMAPBodypart	*FindPartWithNumber(const char *partNum) MOZ_OVERRIDE;	// Returns the part object with the given number

nit: could you fix the indentation, and put the comment on the previous line please?

@@ +176,5 @@
>  	virtual ~nsIMAPBodypartMessage();
> +    virtual int32_t Generate(nsIMAPBodyShell *aShell, bool stream, bool prefetch) MOZ_OVERRIDE;
> +    virtual bool ShouldFetchInline(nsIMAPBodyShell *aShell) MOZ_OVERRIDE;
> +    virtual bool PreflightCheckAllInline(nsIMAPBodyShell *aShell) MOZ_OVERRIDE;
> +	virtual nsIMAPBodypart	*FindPartWithNumber(const char *partNum) MOZ_OVERRIDE;	// Returns the part object with the given number

nit: indentation & comment

::: mailnews/imap/src/nsImapIncomingServer.h
@@ +66,3 @@
>    NS_IMETHOD GetNumIdleConnections(int32_t *aNumIdleConnections);
> +  NS_IMETHOD ForgetSessionPassword() MOZ_OVERRIDE;
> +  NS_IMETHOD GetMsgFolderFromURI(nsIMsgFolder *aFolderResource, const nsACString& aURI, nsIMsgFolder **aFolder) MOZ_OVERRIDE;

nit: line wrapping please
Attachment #755334 - Flags: review?(mozilla) → review+
Comment on attachment 755336 [details] [diff] [review]
Annotate 105 methods with MOZ_OVERRIDE in mailnews/local

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

::: mailnews/local/src/nsRssIncomingServer.h
@@ +27,5 @@
> +    NS_IMETHOD GetOfflineSupportLevel(int32_t *aSupportLevel) MOZ_OVERRIDE;
> +    NS_IMETHOD GetSupportsDiskSpace(bool *aSupportsDiskSpace) MOZ_OVERRIDE;
> +    NS_IMETHOD GetAccountManagerChrome(nsAString& aResult) MOZ_OVERRIDE;
> +    NS_IMETHOD PerformBiff(nsIMsgWindow *aMsgWindow) MOZ_OVERRIDE;
> +    NS_IMETHOD GetServerRequiresPasswordForBiff(bool *aServerRequiresPasswordForBiff) MOZ_OVERRIDE;

nit: line length
Attachment #755336 - Flags: review?(mbanner) → review+
Thanks for doing these Arnaud.
The same one including your comments
Attachment #755332 - Attachment is obsolete: true
Attachment #765396 - Flags: review?(mbanner)
The same one including your comments
Attachment #755333 - Attachment is obsolete: true
Attachment #765398 - Flags: review?(mbanner)
The same one including your comments
Attachment #755334 - Attachment is obsolete: true
Attachment #765400 - Flags: review?(mbanner)
The same one including your comments
Attachment #755336 - Attachment is obsolete: true
Attachment #765401 - Flags: review?(mbanner)
The last one with your previous comments

My pleasure :)
Attachment #754389 - Attachment is obsolete: true
Attachment #765403 - Flags: review?(mbanner)
Attachment #765396 - Flags: review?(mbanner) → review+
Attachment #765398 - Flags: review?(mbanner) → review+
Attachment #765400 - Flags: review?(mbanner) → review+
Attachment #765401 - Flags: review?(mbanner) → review+
Attachment #765403 - Flags: review?(mbanner) → review+
Attachment #764033 - Flags: review?(Pidgeot18) → review+
Comment on attachment 764034 [details] [diff] [review]
Annotate 196 methods with MOZ_OVERRIDE in mailnews/addrbook + fix indentation

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

Looks OK to me.
Attachment #764034 - Flags: review?(mconley) → review+
this patch is the same but we are able to merge it with last thunderbird repo version
Attachment #765398 - Attachment is obsolete: true
Attachment #775326 - Flags: review+
this patch is the same but we are able to merge it with last thunderbird repo version
Attachment #764033 - Attachment is obsolete: true
Attachment #775327 - Flags: review+
Seriously, mark old patches obsolete when you post newer ones. Not doing so makes it a nightmare for someone else trying to help land things for you.
You need to log in before you can comment on or make changes to this bug.