Status

Add-on SDK
General
P1
normal
RESOLVED WONTFIX
6 years ago
5 years ago

People

(Reporter: irakli, Assigned: irakli)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

There are cases where having application wide modules would be way more convenient / efficient then copies of same modules shipped with each add-on. One good example is described under Bug 644595. Such modules also have a potential of defining clear path for moving parts of sdk to a firefox once they are ready.
Assignee: nobody → rFobic

Updated

6 years ago
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
Depends on: 702835
Created attachment 581333 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/290

Pointer to Github pull-request
Attachment #581333 - Flags: review?(poirot.alex)
This is work in progress patch!
So it is not ready for review, right?

In current state, it looks like a major rewrite of all codebase in order to use base/namespace ? Such patch is going to be quite huge and scary! 
Do you plan to split such work ?
(In reply to Alexandre Poirot (:ochameau) from comment #3)
> So it is not ready for review, right?
> 
> In current state, it looks like a major rewrite of all codebase in order to
> use base/namespace ? Such patch is going to be quite huge and scary! 
> Do you plan to split such work ?

It's actually not as big as it currently looks as it includes changes from the other patches that have their own reviews. Once those land change will be much smaller. That being said, yes I did rewrote part that were depending on the xpcom module I've changed but I tried to keep those changes small.
(In reply to Alexandre Poirot (:ochameau) from comment #3)
> So it is not ready for review, right?

Yes I'm still working on it. It does not yet does what description of this bug says. That being said if you'd like to reduce review scope, we could separate required XPCOM module changes into separate bug and review that individually.
Created attachment 582095 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305

Pointer to Github pull-request
Created attachment 582096 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305#

Pointer to Github pull-request
Comment on attachment 582096 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305#

This patch is smaller part of the complete change that should be easier to review.
Attachment #582096 - Flags: review?(poirot.alex)
Attachment #582095 - Attachment is obsolete: true
Comment on attachment 582096 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305#

I put various comments in the pull request that may or may not need modifications

But something is wrong with Factory.createInstance method that needs to be fixed:
https://github.com/mozilla/addon-sdk/pull/305#r309212

Otherwise you forgot to update xpcom documentation.
I'm not against modifying it in a next step as soon as it lands in the same release.
Attachment #582096 - Flags: review?(poirot.alex) → review-
Created attachment 588831 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305

Pointer to Github pull-request
Comment on attachment 588831 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305

Ready for another round!
Attachment #588831 - Flags: review?(poirot.alex)
Comment on attachment 588831 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/305

If you decide to go with solution described here, r+ whatever name you end up choosing  ("from", "for", ...):
https://github.com/mozilla/addon-sdk/pull/305#issuecomment-3579300

Otherwise, please request another review if you use another pattern.
Attachment #588831 - Flags: review?(poirot.alex) → review+

Comment 14

6 years ago
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/aae9630495046e064173948933b9165b38c1dfc3
Merge pull request #305 from Gozala/bug/xpcom-706590

fix bug 706590 - API for implementing XPCOM interfaces r=@ochameau

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
It's not fixed yet, it's just part of this code has landed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.