393951 - Remove appID from nsIExtensionManager.getIncompatibleItemList

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Alex Vincent [:WeirdAl]]

Spin off of bug 299716 - we have an unused argument in getIncompatibleItemList.

Comment 1

[Ian van den Berg]

Attachment: 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.

Comment 2

[Alex Vincent [:WeirdAl]]

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.

Comment 3

[Ian van den Berg]

Attachment: 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.

Comment 4

[Alex Vincent [:WeirdAl]]

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.

Comment 5

[Ian van den Berg]

Thank you, I'm only learning the process of submitting patches. first of many ;)

Comment 6

[Dave Townsend [:mossop]]

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.

Comment 7

[Alex Vincent [:WeirdAl]]

Just out of curiousity, what steps (if any) have you taken to test this patch?

Comment 8

[Robert Strong (they/them - no direct email)]

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

Looks good and thanks!

Comment 9

[Alex Vincent [:WeirdAl]]

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 10

[Robert Strong (they/them - no direct email)]

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

Comment 11

[:Gavin Sharp [email: [e-mail]]]

There's a new review policy, see  http://www.mozilla.org/hacking/reviewers.html . This patch requires superreview.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

(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.

Comment 13

[Robert Strong (they/them - no direct email)]

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

Comment 14

[Robert Strong (they/them - no direct email)]

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

Comment 15

[Ian van den Berg]

Thanks, I'll definitely say hi. 
one down how many to go? ;)