717119 - Typo in documentation comment for AddonUpdateChecker.getNewestCompatibleUpdate()

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonUpdateChecker.jsm#710

One of these things should not be like the other.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Taking this as a [good first bug] for the MSU Capstone team.

Comment 2

[MSU Capstone Team]

Our account is up and ready to go.

Comment 3

[MSU Capstone Team]

Attachment: Typo has been fixed

So the way we created this patch, we think is correct but may not be.  First we used hg qimport to import the file AddonUpdateChecker.jsm into out /.hg/patches folder.  We then edited that file to get rid of the typo.  After that we ran the command hg diff on that file to create a patch we named Test_Patch.  We think this is correct but are not sure, the hg import command is supposed to create a patch but that part was unclear to us.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to MSU Mozilla from comment #3)
> Created attachment 588158 [details] [diff] [review]
> Typo has been fixed
> 
> So the way we created this patch, we think is correct but may not be.  First
> we used hg qimport to import the file AddonUpdateChecker.jsm into out
> /.hg/patches folder.  We then edited that file to get rid of the typo. 
> After that we ran the command hg diff on that file to create a patch we
> named Test_Patch.  We think this is correct but are not sure, the hg import
> command is supposed to create a patch but that part was unclear to us.

hg qimport will import a patch that somebody else made that you want to put in to your local patch queue, which you can then apply to your tree.

Here's what you should do:
1) Find the file in your tree at /toolkit/mozapps/extensions/AddonUpdateChecker.jsm
2) Change the necessary lines in that file.
3) Now run 'hg diff'
3a) That should show you just the lines that you changed and a little bit of context.
4) You can now do 'hg diff > 717119.patch' to export your diff.
5) The preferred way going forward is to instead do the following:
5a) Change directories to /mozilla-central/ and run 'hg qinit'
5b) Run 'hg qnew -m "Bug 717119 - Description of patch" 717119.patch
6) Your patch is now located at /mozilla-central/.hg/patches/717119.patch
7) Upload the 717119.patch file to this bug and request review from Blair.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

For a more detailed description of what to do, you can follow these instructions: https://developer.mozilla.org/en/Creating_a_patch

Comment 6

[MSU Capstone Team]

(In reply to Jared Wein [:jwein and :jaws] from comment #5)
> For a more detailed description of what to do, you can follow these
> instructions: https://developer.mozilla.org/en/Creating_a_patch

Ok that's what I initially thought we had to do, but our file AddonUpdateCheck.jsm did not have the typo that was in the file in Blair's link so I thought we might have to download his file.  How do you want us to proceed because we have no typo in our file?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to MSU Mozilla from comment #6)
> (In reply to Jared Wein [:jwein and :jaws] from comment #5)
> > For a more detailed description of what to do, you can follow these
> > instructions: https://developer.mozilla.org/en/Creating_a_patch
> 
> Ok that's what I initially thought we had to do, but our file
> AddonUpdateCheck.jsm did not have the typo that was in the file in Blair's
> link so I thought we might have to download his file.  How do you want us to
> proceed because we have no typo in our file?

Hmmm ... it should be there. The linewas added on December 2nd as part of this changeset: http://hg.mozilla.org/mozilla-central/rev/ac0b65079106

If you do a search in that file for |aIgnoreMaxVersion|, do you see |aIgnoreMaxVersion| at line 712?

Comment 8

[MSU Capstone Team]

Attachment: Typo fixed for real

Ok, I somehow was looking at the wrong function, I was looking at getCompatibleUpdate() not getNewestCompatibleUpdate(), so the typo was there and we fixed it.  I was able to create the diff using the command 'hg diff > 717119.patch'.  However when I tried using the command from your instructions in step 5b) it told me I needed to supply a username and I did not know how to proceed.  So the patch was created it just wasn't placed in the /.hg/patches folder.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 588476 [details] [diff] [review]
Typo fixed for real

Thanks for the patch. Can you please follow the steps in Marco's post about how to make your patch ready for check-in?

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Jared Wein [:jwein and :jaws] from comment #9)
> Comment on attachment 588476 [details] [diff] [review]
> Typo fixed for real
> 
> Thanks for the patch. Can you please follow the steps in Marco's post about
> how to make your patch ready for check-in?

Sorry, the URL for Marco's post got lost. Here it is: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 588476 [details] [diff] [review]
Typo fixed for real

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

(Waiting on updated patch as described in comment 10)

Comment 12

[MSU Capstone Team]

Attachment: Bug-717119 fixed. F

First patch by MSU Team Mozilla. Please review.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 589556 [details] [diff] [review]
Bug-717119 fixed. F

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

This is good. Normally the summary for the bug is formatted like so:
> Bug 717119 - Typo in documentation comment for AddonUpdateChecker.getNewestCompatibleUpdate().

Then the 'r=bmcbride' part is added after it is reviewed, so the final summary before landing would be:
> Bug 717119 - Typo in documentation comment for AddonUpdateChecker.getNewestCompatibleUpdate(). r=bmcbride

Thanks for the patch!

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Landed on the Fx-team branch, which gets merged into the mozilla-central branch every day or two.

https://hg.mozilla.org/integration/fx-team/rev/d2b7fa944d77

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/d2b7fa944d77