1008199 - Factor out unneeded dependencies in functional.js

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Eddy Bruel [:ejpbruel]]

protocol.js in the debugger server has an indirect dependency on functional.js in the SDK.

functional.js, in turn, has two dependencies that cause the SDK loader to be required. This is somewhat of a problem for me, because the worker loader I'm implementing in bug 1003095 is currently unable to handle loading the SDK loader.

However, it turns out that the parts of functional.js that are used by protocol.js actually do not need those two dependencies. I therefore propose a patch that does the following:

- Drop the deprecated function chain
- Either move the calls to require("../timer") into the function that need them, or split out the functions that don't need access to timers into a separate file.

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: Pointer to pull request

It's been a long time since I last made a pull request to the add-on SDK repo, so please let me know if I've done anything wrong.

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 8423107 [details]
Pointer to pull request

See comments in the pull request.

P.S.: If you just a put pull request url in the attachment, bugzilla will do a right thing.

Comment 3

[Eddy Bruel [:ejpbruel]]

Attachment: Pointer to pull request

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 8423422 [details] [review]
Pointer to pull request

Can you please address one more thing before you land this:

We usually put sub-components under the folder that is named as a component they implement. So instead of having functional-core.js, functional-concurrent.js we would have lang/functional/core.js, functional/concurrent.js...

I don't think we need another review for that though.

Thanks!

Comment 5

[Eddy Bruel [:ejpbruel]]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #4)
> Comment on attachment 8423422 [details] [review]
> Pointer to pull request
> 
> Can you please address one more thing before you land this:
> 
> We usually put sub-components under the folder that is named as a component
> they implement. So instead of having functional-core.js,
> functional-concurrent.js we would have lang/functional/core.js,
> functional/concurrent.js...
> 
> I don't think we need another review for that though.
> 
> Thanks!

Sure! I always prefer r+ with comments addressed over r- ;-)

Thanks for the quick turnover time!

Comment 6

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Eddy Bruel [:ejpbruel] from comment #1)

> It's been a long time since I last made a pull request to the add-on SDK
> repo, so please let me know if I've done anything wrong.

Sorry Eddy, I was in a conference in Italy when you ask to review this patch, glad Irakli did it.

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/ef2753ffc0053583c423615a4f5ef06f0a37ac7e
Bug 1008199 - Factor out unneeded dependencies in functional.js

https://github.com/mozilla/addon-sdk/commit/ebcac059898459e0b3cb65df615c57d576803f59
Merge pull request #1488 from ejpbruel/1008199

fix Bug 1008199 - Factor out unneeded dependencies in functional.js r=@gozala