Address book comment cleanup

RESOLVED FIXED in mozilla1.9.1b2

Status

defect
--
trivial
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: gkw, Assigned: gkw)

Tracking

Trunk
mozilla1.9.1b2
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

I was going through the comments in the address book directory of mailnews and thought to do a linguistic cleanup at the same time..
Attachment #342194 - Flags: review?(bugzilla)
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.
Attachment #342194 - Flags: review?(bugzilla) → review+
Bringing over review+.
Attachment #342194 - Attachment is obsolete: true
Attachment #342427 - Flags: review+
Posted patch patch2.1 (obsolete) — Splinter Review
Oops, carelessness crept in.
Attachment #342427 - Attachment is obsolete: true
Attachment #342429 - Flags: review+
Attachment #342427 - Flags: review+
Comment on attachment 342429 [details] [diff] [review]
patch2.1

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

updated one coming soon...
Attachment #342429 - Flags: review+
Attachment #342429 - Attachment is obsolete: true
How about proper capitalization for sentences, while you're at it? ;)
Posted patch patch v3 (obsolete) — Splinter Review
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.
Attachment #342541 - Flags: review+
(In reply to comment #6)
> in these cases nothing was changed.

... in these cases nothing "related to capitalization" was changed.
Keywords: checkin-needed
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.
Attachment #342541 - Attachment is obsolete: true
Attachment #342546 - Flags: review?(bugzilla)
Attachment #342541 - Flags: review+
Keywords: checkin-needed
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.
Attachment #342546 - Flags: review?(bugzilla) → review+
This patch has been un-bit-rotted, and also addresses the nit in the previous comment.
Attachment #342546 - Attachment is obsolete: true
Attachment #343398 - Flags: review+
forgot to note that I also brought over the review+, now adding checkin-needed.
Keywords: checkin-needed
Comment on attachment 343398 [details] [diff] [review]
patch v5 addressing comments
[Checkin: Comment 12]

http://hg.mozilla.org/comm-central/rev/74eeeb2edbe1
Attachment #343398 - Attachment description: patch v5 addressing comments → patch v5 addressing comments [Checkin: Comment 12]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
You need to log in before you can comment on or make changes to this bug.