Blair McBride [:Unfocused] (UNAVAILABLE) Reporter
	Description • 14 years ago

Given that AddonsManager has getAddonsByTypes() and getAddonsWithPendingOperations(), the name of getAddons() is a bit misleading. It implies that it will get all addons, not restricted by any criteria - when what is really wanted is getAddonsByTypes(null, ...).

I'd like to see this renamed to getAddonsByIDs().

As an alternate (or in addition to!) solution, getAddons() could support a null first-argument similar to getAddonsByTypes().

	Dave Townsend [:mossop] Assignee
	Comment 1 • 14 years ago

Changing getAddons to getAddonsByID and getAddon to getAddonByID seems to make things consistent. Do you agree with this Rob?

	Robert Strong (they/them - no direct email)
	Comment 2 • 14 years ago

I kind of prefer getAddonsByIDs over getAddonsByID to be consistent with the names of similar functions. Other than that I agree.

	Blair McBride [:Unfocused] (UNAVAILABLE) Reporter
	Comment 3 • 14 years ago

See also bug 556200.

	Robert Strong (they/them - no direct email)
	Comment 4 • 14 years ago

If this is to be done then getAddon should probably be renamed to getAddonByID. With the ID being the default property to identify an add-on I don't have a serious issue with the name being getAddons but I do agree with Blair that getAddonsByIDs is clearing just as getAddonByID is also clearer.

	Dave Townsend [:mossop] Assignee
	Comment 5 • 14 years ago

So I think to be consistent we should switch everything to the following methods:

getAddonByID
getAddonsByIDs
getAddonsByTypes
getAddonsWithPendingOperationsByTypes (urk!)
getInstallsByTypes

I'm also tempted to add an explicit:

getAllAddons
getAllInstalls

How does this sound?

	Robert Strong (they/them - no direct email)
	Comment 6 • 14 years ago

Sounds good. My initial impression of getAddonsWithPendingOperationsByTypes is that it could be renamed to getAddonsWithOperationsByTypes. Pernding is redundant since if it has an operation to perform the operation is a pending operation.

	Dave Townsend [:mossop] Assignee
	Comment 7 • 14 years ago

Pure style changes to add prefixes to all arguments and fix spacing in javadoc comments. Rob says this does not require review.

	Dave Townsend [:mossop] Assignee
	Comment 8 • 14 years ago

Comment on attachment 439321 [details] [diff] [review]
style changes

This is wrong

	Dave Townsend [:mossop] Assignee
	Comment 10 • 14 years ago

These are just the naming changes, not sure if you need to do much of a review of the patch.

	Robert Strong (they/them - no direct email)
	Comment 11 • 14 years ago

Comment on attachment 439432 [details] [diff] [review]
naming changes rev 1

looks good!

	Dave Townsend [:mossop] Assignee
	Comment 12 • 14 years ago

Landed:

http://hg.mozilla.org/projects/addonsmgr/rev/aeae9f36e2d2
http://hg.mozilla.org/projects/addonsmgr/rev/847ffd15dbdb

All the APIs are covered by automated tests.

	Dave Townsend [:mossop] Assignee
	Comment 13 • 14 years ago

Missed a couple of uses in the plugins tests

	Dave Townsend [:mossop] Assignee
	Comment 14 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/adfa8cb1c5ec
http://hg.mozilla.org/mozilla-central/rev/f63f946b292f

	Dave Townsend [:mossop] Assignee
	Comment 15 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/67c5cfbb4ba9

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 16 • 14 years ago

Verified fixed by hg log.