459000 - Address book comment cleanup

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch of mostly address book comment fixes

I was going through the comments in the address book directory of mailnews and thought to do a linguistic cleanup at the same time..

Comment 1

[Mark Banner (:standard8)]

Comment on attachment 342194 [details] [diff] [review]
patch of mostly address book comment fixes

>-    // PR_TRUE (ie, case insensitive) without reading bugs #128535 and #121478.
>+    // PR_TRUE (i.e., case insensitive) without reading bugs #128535 and #121478.

nit: there is no need for the comma here.

>-        // donot check the return here since delete may fail if 
>-        // entry deleted in changelog doesnot exist in DB 
>+        // do not check the return here since delete may fail if 
>+        // entry deleted in changelog does not exist in DB 

nit: as you're touching them please remove the extra whitespace from the end of the lines.

>-    // eg : in case a filter is specified.
>+    // e.g. : in case a filter is specified.

nit: drop the colon, its not necessary.

>   NS_DECL_ISUPPORTS
> 
> 	//////////////////////////////////////////////////////////////////////////
>-	// we suppport the nsIProtocolHandler interface 
>+	// we support the nsIProtocolHandler interface 
> 	//////////////////////////////////////////////////////////////////////////

Might as well drop the tabs from these four lines whilst your here, and the whitespace at the end of the line.

> NS_IMETHODIMP 
> nsLDAPAutoCompleteSession::OnAutoComplete(const PRUnichar *searchString, 
>                                  nsIAutoCompleteResults *previousSearchResult, 
>                                           nsIAutoCompleteListener *listener)
> {
>     // OnStopLookup should have already been called, so there's nothing to
>-    // free here.  Additionally, as of this writing, noone is hanging around 
>+    // free here.  Additionally, as of this writing, no one is hanging around 

nit: whitespace at end of line again.

r=me with that lot fixed.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch2 with comments addressed

Bringing over review+.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch2.1

Oops, carelessness crept in.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 342429 [details] [diff] [review]
patch2.1

bah, this doesn't merge properly. my bad.

updated one coming soon...

Comment 5

[Magnus Melin [:mkmelin]]

How about proper capitalization for sentences, while you're at it? ;)

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch v3

Done up v3 off another |hg diff|, addressing Standard8's comments and capitalizing some sentences. Bringing over review+.

/me notes that some comments always start with a lowercase in files such as mailnews/addrbook/src/nsAbLDAPChangeLogData.cpp, in these cases nothing was changed.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #6)
> in these cases nothing was changed.

... in these cases nothing "related to capitalization" was changed.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch v4 with capitalization corrections

So philor reminds me that the capitalization corrections are recommended, so here they are.

Requesting review again due to the large number of changes over the previous reviewed patch.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 342546 [details] [diff] [review]
patch v4 with capitalization corrections

nsAbAutoCompleteSession.cpp is now dead.

+  // We do that, we can fix this code.  at best, it will turn a sort into a invalidate.

You missed capitalising the a of at.

r=me with those fixed.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch v5 addressing comments [Checkin: Comment 12]

This patch has been un-bit-rotted, and also addresses the nit in the previous comment.

Comment 11

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

forgot to note that I also brought over the review+, now adding checkin-needed.

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 343398 [details] [diff] [review]
patch v5 addressing comments
[Checkin: Comment 12]

http://hg.mozilla.org/comm-central/rev/74eeeb2edbe1