Skip to content

Conversation

muffinresearch
Copy link
Contributor

Changes the addon manager to not be a class.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 219eb16 on muffinresearch:refactor-addonmanager into a665343 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 566c927 on muffinresearch:refactor-addonmanager into a665343 on mozilla:master.

return _mozAddonManager.getAddonByID(guid)
.then((addon) => {
if (!addon) {
return Promise.reject(new Error('Addon not found'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just moved this but I think this could throw/return instead.

if (!addon) {
  throw new Error('Addon not found');
}
return addon;

@muffinresearch muffinresearch force-pushed the refactor-addonmanager branch from 566c927 to bf89920 Compare June 6, 2016 18:11
if (!addon) {
throw new Error('Addon not found');
}
return Promise.resolve(addon);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just return addon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep - good catch!

@mstriemer
Copy link
Contributor

r+wc

Thanks!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling bf89920 on muffinresearch:refactor-addonmanager into b08ad9f on mozilla:master.

@muffinresearch muffinresearch force-pushed the refactor-addonmanager branch from bf89920 to a19ae44 Compare June 6, 2016 19:03
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a19ae44 on muffinresearch:refactor-addonmanager into 3bd2af9 on mozilla:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants