Remove appID from nsIExtensionManager.getIncompatibleItemList

RESOLVED FIXED in mozilla1.9.2a1

Status

()

--
minor
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: WeirdAl, Assigned: ian.vdberg)

Tracking

unspecified
mozilla1.9.2a1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Spin off of bug 299716 - we have an unused argument in getIncompatibleItemList.
Summary: Remove id from nsIExtensionManager.getIncompatibleItemList → Remove appID from nsIExtensionManager.getIncompatibleItemList
Product: Firefox → Toolkit
Whiteboard: [good first bug]
(Assignee)

Comment 1

9 years ago
Created attachment 391441 [details] [diff] [review]
Patch for  Bug 393951 : Removed appID from nsIExtensionManager.getIncompatibleItemList

This is a patch file, I removed appID from nsIExtensionManager.getIncompatibleItemList and everywhere it was used. 

This is my first patch, I want to get into the open source bug fixing world.
Attachment #391441 - Flags: review?(gavin.sharp)
(Reporter)

Comment 2

9 years ago
Comment on attachment 391441 [details] [diff] [review]
Patch for  Bug 393951 : Removed appID from nsIExtensionManager.getIncompatibleItemList

>diff -r ad3c5bd01693 toolkit/mozapps/extensions/public/nsIExtensionManager.idl
...
>-  void getIncompatibleItemList(in AString id,
>-                               in AString appVersion,
>+  void getIncompatibleItemList(in AString appVersion,

Nit:  you'll need to change the UUID of the interface.
(Assignee)

Comment 3

9 years ago
Created attachment 391481 [details] [diff] [review]
Patch for  Bug 393951 : added change in uuid for the interface

new patch. I've added the change in the UUID of the interface nsIExtensionManager.
Attachment #391481 - Flags: review?(ajvincent)
(Reporter)

Updated

9 years ago
Attachment #391441 - Attachment is obsolete: true
Attachment #391441 - Flags: review?(gavin.sharp)
(Reporter)

Comment 4

9 years ago
Comment on attachment 391481 [details] [diff] [review]
Patch for  Bug 393951 : added change in uuid for the interface

:) I'm not sure I should review this.  It's been a long time since I touched EM code (though admittedly this patch is trivial).

Generally speaking, when you submit a newer version of a patch, you obsolete the older one at the same time.  FYI.

I'll hand this review request over to Mossop, who owns EM these days.
Attachment #391481 - Flags: review?(ajvincent) → review?(dtownsend)
(Assignee)

Comment 5

9 years ago
Thank you, I'm only learning the process of submitting patches. first of many ;)
Comment on attachment 391481 [details] [diff] [review]
Patch for  Bug 393951 : added change in uuid for the interface

(In reply to comment #5)
> Thank you, I'm only learning the process of submitting patches. first of many
> ;)

Good to hear. As a general rule check the module owners list to see peers that are capable of reviewing for the area.

Going to pass this over to Rob since I'm on vacation, if he doesn't get to it by the time I get back though I'll do it.
Attachment #391481 - Flags: review?(dtownsend) → review?(robert.bugzilla)
(Reporter)

Comment 7

9 years ago
Just out of curiousity, what steps (if any) have you taken to test this patch?
Comment on attachment 391481 [details] [diff] [review]
Patch for  Bug 393951 : added change in uuid for the interface

Looks good and thanks!
Attachment #391481 - Flags: review?(robert.bugzilla) → review+
(Reporter)

Comment 9

9 years ago
FYI: in some areas a patch needs super-review, but Toolkit isn't one of them, I think, so this is ready to check in.  Do you have permissions to do so, Ian?  (I don't.)
Comment on attachment 391481 [details] [diff] [review]
Patch for  Bug 393951 : added change in uuid for the interface

The rules recently changed so let me verify whether not not this will need sr before landing
Attachment #391481 - Flags: superreview?(robert.bugzilla)
There's a new review policy, see  http://www.mozilla.org/hacking/reviewers.html . This patch requires superreview.
(In reply to comment #10)
> (From update of attachment 391481 [details] [diff] [review])
> The rules recently changed so let me verify whether not not this will need sr
> before landing

Oops, sorry, hadnt seen this comment before replying.
Attachment #391481 - Flags: superreview?(robert.bugzilla) → superreview?(vladimir)
Comment on attachment 391481 [details] [diff] [review]
Patch for  Bug 393951 : added change in uuid for the interface

Vlad, this removes an unused param from em's getIncompatibleItemList and hence needs an sr
Attachment #391481 - Flags: superreview?(vladimir) → superreview+
Thanks for the patch Ian

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/c846a65b9c62

note: the call to getIncompatibleItemList is performed in several tests so marking in-testsuite

Ian, if you are ever on irc say hi. I'm rs on irc and Dave is Mossop. Cheers
Assignee: nobody → ian.vdberg
Flags: in-testsuite+
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug]
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Comment 15

9 years ago
Thanks, I'll definitely say hi. 
one down how many to go? ;)
You need to log in before you can comment on or make changes to this bug.