Closed Bug 1314176 Opened 8 years ago Closed 7 years ago

convert AddonManager to ES6 classes

Categories

(Toolkit :: Add-ons Manager, defect, P3)

defect

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

(Whiteboard: triaged)

Attachments

(1 file, 1 obsolete file)

The AddonManager code in toolkit/mozapps/extensions defines many custom objects using prototype-based OO, we could reduce the amount of code and make these more readable by using ES6 classes instead.
\o/
Depends on: 1185106
Blocks: 1314177
Converting to classes requires rewriting Task.async over to Task.span (plus binding (this)), so let's just convert over to async/await while we're at it.
Summary: convert AddonManager to ES6 classes → convert AddonManager to ES6 classes and Task.* to ES7 async/await
(In reply to Robert Helmer [:rhelmer] from comment #2)
> Converting to classes requires rewriting Task.async over to Task.span

Or Task.spawn, if you want it to actually work ;)
Just a heads up - I was asking around in #jsapi, and while async/await is enabled for all branches, they suggest we wait a cycle before doing mass conversions to let the feature percolate. Waldo said we should try to avoid a String.prototype.contains-type snafu.
(In reply to Mike Conley (:mconley) - (digging out of review / needinfo backlog) from comment #5)
> Just a heads up - I was asking around in #jsapi, and while async/await is
> enabled for all branches, they suggest we wait a cycle before doing mass
> conversions to let the feature percolate. Waldo said we should try to avoid
> a String.prototype.contains-type snafu.

Makes sense, I can just keep a branch rebased and ready to go for when we feel comfortable.
Comment on attachment 8806446 [details]
Bug 1314176 - convert AddonManager from Task.{async,spawn} to async/await

Actually don't bother reviewing this yet - I see some test failures, and I can simplify this further too.
Attachment #8806446 - Flags: review?(kmaglione+bmo)
Depends on: 1315407
marking triaged and P3 - as rhelmer already took.  priority could be changed - but setting removes from untriaged list.
Priority: -- → P3
Whiteboard: triaged
Since classes are ready to use now and we're holding off on async/await, let's just switch to classes now and do the Task.* -> async/await switch later.
No longer depends on: 1185106
Summary: convert AddonManager to ES6 classes and Task.* to ES7 async/await → convert AddonManager to ES6 classes
Attachment #8806446 - Attachment is obsolete: true
Comment on attachment 8810437 [details]
Bug 1314176 - convert AddonManager to ES6 classes

Removing r? while I work through some test failures.
Attachment #8810437 - Flags: review?(aswan)
After looking at this more, I think it's probably not worth doing this as one big change. It'd make more sense to do things as-needed like bug 1314177, especially considering that we should be able to remove a fair amount of code post-57.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.