Fix bad usage of "it's" (and other misc typos) in comments

RESOLVED FIXED in mozilla13

Status

()

Core
General
--
trivial
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(8 attachments, 2 obsolete attachments)

64.22 KB, patch
Details | Diff | Splinter Review
20.08 KB, patch
jst
: review+
Details | Diff | Splinter Review
4.86 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
1.96 KB, patch
bsmedberg
: review+
Details | Diff | Splinter Review
5.27 KB, patch
jst
: review+
Details | Diff | Splinter Review
13.35 KB, patch
bsmedberg
: review+
Details | Diff | Splinter Review
5.85 KB, patch
Details | Diff | Splinter Review
34.15 KB, patch
Ehsan
: review+
Details | Diff | Splinter Review
Created attachment 341406 [details] [diff] [review]
"it's" fixes patch 1: misc [landed 2008/10/01]

After noticing an instance of the possessive "its" being misspelled as "it's", I was inspired to clean up these sorts of typos on a large scale.

I'm filing this bug to manage these typo-fixes -- to have a bug to mention in the checkin comments, and to have a place for people to comment in case they dispute one of the changes. :)

I'm attaching "volume 1" of fixes, which covers these subdirectories (or at least, the bits of them that get checked out with mozilla-central):
  accessible
  browser
  config
  profile
  rdf
  toolkit
  tools
  xpcom
  xpfe
Just landed vol1 (attachment 341406 [details] [diff] [review]) as http://hg.mozilla.org/mozilla-central/rev/d47a01a87c6c
Its great you took the time to fix these!

Comment 3

9 years ago
My personal opinion is that patches like this shouldn't land without module owner consent (to avoid busting existing patches), and clearly not without review. And not on a closed tree either.
> and clearly not without
> review. And not on a closed tree either.

Hmm... I've been told multiple times that minor, obvious comment-only typo-fixes don't need review.

I also thought I'd been told that such changes are considered NPOTB and are OK to land even during minor closures, as long as it's cool by the sheriff.  In this particular case, sheriff was #developers, and no one in #developers objected when I asked about this.  (Also, the tree was green, and this particular closure was just "for various landings" which AFAIK had already been landed.)  Also, there had already been a number of earlier whitespace-only commits to trigger builds (e.g. changeset fff6e8b4d30f), and my commit didn't seem very different from those.

> My personal opinion is that patches like this shouldn't land without module
> owner consent (to avoid busting existing patches)

That's a fair point, regarding merge issues in outstanding patches --  I'll make sure to check with relevant module-owners / peers before landing future volumes in this family of patches. :)

(I'm also happy to back the existing patch out, if any module owner would prefer that I postpone these typo-fixes in their module until after something lands...  but otherwise, I'd lean towards leaving it, because I think we're better with the fixed comments. :))

Comment 5

9 years ago
Comment on attachment 341406 [details] [diff] [review]
"it's" fixes patch 1: misc [landed 2008/10/01]

>-  // that it's operation be reversable from Finish.
>+  // that its operation be reversable from Finish.
reversible ;-)
Created attachment 358189 [details] [diff] [review]
"it's" fixes patch 2: content [landed 2009/01/23]
Created attachment 358190 [details] [diff] [review]
"it's" fixes patch 3: db  [landed 2009/01/23]
Attachment #358190 - Attachment description: it's" fixes patch 2: db folder (db/mdb/, db/mork) → "it's" fixes patch 3: db folder (db/mdb/, db/mork)
Created attachment 358191 [details] [diff] [review]
"it's" fixes patch 4: docshell [landed 2009/02/05]
Created attachment 358192 [details] [diff] [review]
"it's" fixes patch 5: dom  [landed 2009/01/23]
Created attachment 358193 [details] [diff] [review]
"it's" fixes patch 6: editor folder
Created attachment 358194 [details] [diff] [review]
"it's" fixes patch 7: embedding [landed 2009/02/05]
Attachment #358194 - Attachment description: "it's" fixes patch 6: embedding folder → "it's" fixes patch 7: embedding folder
Created attachment 358195 [details] [diff] [review]
"it's" fixes patch 8: extensions folder
Created attachment 358196 [details] [diff] [review]
"it's" fixes patch 9: gfx [mailed to cairo]
Requesting approval from module owners this time around, per comment 3 & comment 4. :)
Attachment #358189 - Flags: review?(jst)
Attachment #358192 - Flags: review?(jst)
Attachment #358190 - Flags: review?(sdwilsh)
Attachment #358191 - Flags: review?(benjamin)
Attachment #358193 - Flags: review?(daniel)
Attachment #358194 - Flags: review?(benjamin)
Attachment #358196 - Flags: review?(vladimir)
Comment on attachment 358196 [details] [diff] [review]
"it's" fixes patch 9: gfx [mailed to cairo]

vlad -- these typo-fixes are all in gfx/cairo/cairo -- is it useful to make changes there, or does that just get overwritten every time we do a cairo upgrade?

Updated

9 years ago
Attachment #358190 - Flags: review?(sdwilsh) → review?(Pidgeot18)
Comment on attachment 358190 [details] [diff] [review]
"it's" fixes patch 3: db  [landed 2009/01/23]

Seems like McCusker doesn't know his possessive from his contraction...
Attachment #358190 - Flags: review?(Pidgeot18) → review+
Comment on attachment 358196 [details] [diff] [review]
"it's" fixes patch 9: gfx [mailed to cairo]

It gets overwritten.  But send the patch to the cairo mailing list, I'm sure it'll get taken upstream. (cairo@cairographics.org)

Updated

9 years ago
Attachment #358189 - Flags: superreview+
Attachment #358189 - Flags: review?(jst)
Attachment #358189 - Flags: review+

Updated

9 years ago
Attachment #358192 - Flags: superreview+
Attachment #358192 - Flags: review?(jst)
Attachment #358192 - Flags: review+
Attachment #358196 - Attachment is patch: true
Attachment #358196 - Flags: review?(vladimir)
Comment on attachment 358196 [details] [diff] [review]
"it's" fixes patch 9: gfx [mailed to cairo]

(In reply to comment #17)
> But send the patch to the cairo mailing list

Ok, thanks -- I just sent the patch to the cairo list.
Attachment #358196 - Attachment description: "it's" fixes patch 9: gfx folder (gfx/cairo/cairo) → "it's" fixes patch 9: gfx/cairo/cairo [mailed upstream]
Attachment #341406 - Attachment description: "it's" fixes vol1 → "it's" fixes vol1 [checked in 2008/10/01]
Landed patches #2, #3, #5 (for content, db, and dom folders):
http://hg.mozilla.org/mozilla-central/rev/c6dd3fd7c949
http://hg.mozilla.org/mozilla-central/rev/4d8381e446c4
http://hg.mozilla.org/mozilla-central/rev/edb61a05b485
Attachment #358189 - Attachment description: "it's" fixes patch 2: content folder → "it's" fixes patch 2: content [landed 2009/01/23]
Attachment #358190 - Attachment description: "it's" fixes patch 3: db folder (db/mdb/, db/mork) → "it's" fixes patch 3: db [landed 2009/01/23]
Attachment #358192 - Attachment description: "it's" fixes patch 5: dom folder → "it's" fixes patch 5: dom [landed 2009/01/23]
Attachment #341406 - Attachment description: "it's" fixes vol1 [checked in 2008/10/01] → "it's" fixes vol1 [landed 2008/10/01]
Attachment #358196 - Attachment description: "it's" fixes patch 9: gfx/cairo/cairo [mailed upstream] → "it's" fixes patch 9: gfx [mailed to cairo]
Attachment #341406 - Attachment description: "it's" fixes vol1 [landed 2008/10/01] → "it's" fixes patch 1: misc [landed 2008/10/01]
Attachment #358191 - Flags: review?(benjamin) → review+
Attachment #358194 - Flags: review?(benjamin) → review+
(In reply to comment #2)
> Its great you took the time to fix these!
  ^^^
  It's

Was this intended as a joke?
(In reply to comment #20)
> (In reply to comment #2)
> > Its great you took the time to fix these!
>   ^^^
>   It's
> 
> Was this intended as a joke?

Ha! Dolske, you're a genius.  I totally missed that. :)
Landed patches #4 & #7 (for docshell and embedding folders):
http://hg.mozilla.org/mozilla-central/rev/b91945d3be91
http://hg.mozilla.org/mozilla-central/rev/00e5873f4641
Attachment #358191 - Attachment description: "it's" fixes patch 4: docshell folder → "it's" fixes patch 4: docshell [landed 2009/02/05]
Attachment #358194 - Attachment description: "it's" fixes patch 7: embedding folder → "it's" fixes patch 7: embedding [landed 2009/02/05]
Created attachment 595544 [details] [diff] [review]
"it's" fixes patch 6: editor folder (v2) [landed 2012-02-15]

Just noticed patch 6 in my bugzilla "my requests" page, awaiting review. :)

Updated patch to only include the chunks that still apply, and requesting review from an active /editor hacker.
Attachment #358193 - Attachment is obsolete: true
Attachment #595544 - Flags: review?(ehsan)
Attachment #358193 - Flags: review?(daniel)
Comment on attachment 358195 [details] [diff] [review]
"it's" fixes patch 8: extensions folder

(obsoleting patch 8, as its (ha!) patched code doesn't exist in m-c anymore)
Attachment #358195 - Attachment is obsolete: true
Attachment #595544 - Flags: review?(ehsan) → review+
Comment on attachment 595544 [details] [diff] [review]
"it's" fixes patch 6: editor folder (v2) [landed 2012-02-15]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1db98753a46f
Attachment #595544 - Attachment description: "it's" fixes patch 6: editor folder (v2) → "it's" fixes patch 6: editor folder (v2) [landed 2012-02-15]
[this can be closed once that last patch is landed on m-c.  hip hip hooray.]
[er s/landed on/merged to/]
https://hg.mozilla.org/mozilla-central/rev/1db98753a46f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.