Closed Bug 717119 Opened 8 years ago Closed 8 years ago

Typo in documentation comment for AddonUpdateChecker.getNewestCompatibleUpdate()

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: Unfocused, Assigned: owenccarpenter)

Details

Attachments

(1 file, 2 obsolete files)

Taking this as a [good first bug] for the MSU Capstone team.
Assignee: nobody → jwein
Our account is up and ready to go.
Assignee: jwein → owenccarpenter
Status: NEW → ASSIGNED
Attached patch Typo has been fixed (obsolete) — Splinter Review
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.
(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.
For a more detailed description of what to do, you can follow these instructions: https://developer.mozilla.org/en/Creating_a_patch
(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?
(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?
Attached patch Typo fixed for real (obsolete) — Splinter Review
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.
Attachment #588158 - Attachment is obsolete: true
Attachment #588158 - Flags: feedback?
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?
Attachment #588476 - Flags: feedback+
Attachment #588476 - Flags: review?(bmcbride)
(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 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)
Attachment #588476 - Flags: review?(bmcbride)
First patch by MSU Team Mozilla. Please review.
Attachment #589556 - Flags: review?
Attachment #589556 - Flags: review? → review?(bmcbride)
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!
Attachment #589556 - Flags: feedback+
Attachment #588476 - Attachment is obsolete: true
Attachment #589556 - Flags: review?(bmcbride) → review+
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
Flags: in-testsuite-
Flags: in-litmus-
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/d2b7fa944d77
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
You need to log in before you can comment on or make changes to this bug.