724632 - (sdk-kill-traits) [tracking] Remove use of traits

Status: RESOLVED FIXED

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

Description

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

Historically there have been many attempts to provide a powerful way for object composition with a decent support for capabilities encapsulation. We should migrate all of this code to Base.extend and namespaces to low the maintenance cost and learning curve.

Comment 1

[Alexandre Poirot [:ochameau]]

Sounds like a meta bug!

I have started some weeks ago to convert Widget API.
I can revive this work if it can help, or not. As you prefer.
(It was mainly an educational work, in order to play with base and ns)
Most of missing pieces was about migrating worker/symbiont APIs.
So if you already rewrote them, I may be able to adapt Widget more easily.

My only concern would be for my patch in bug 718666.
Do you think I should try to land it before this work?

Comment 2

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

(In reply to Alexandre Poirot (:ochameau) from comment #1)
> Sounds like a meta bug!
> 
> I have started some weeks ago to convert Widget API.
> I can revive this work if it can help, or not. As you prefer.
> (It was mainly an educational work, in order to play with base and ns)
> Most of missing pieces was about migrating worker/symbiont APIs.
> So if you already rewrote them, I may be able to adapt Widget more easily.
> 

I have some work in progress branches with that, but had some issues that need to be addressed.

> My only concern would be for my patch in bug 718666.
> Do you think I should try to land it before this work?

Regardless I created this and other similar bugs, to prioritize them and request some working cycles, so they are not landing anytime soon.

Comment 3

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/672

Pointer to Github pull-request

Comment 4

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/671

Pointer to Github pull-request

Comment 5

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/668

Pointer to Github pull-request

Comment 6

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

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

Oops wrong, zer0 :)

Comment 7

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/673

Pointer to Github pull-request

Comment 8

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/674

Pointer to Github pull-request

Comment 9

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

Irakli, just to have a confirm: those pull request are stand alone, right? I can review them separately and they're not dependent by each other (at least, if I start from the lowest pull's number)?

Comment 10

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

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #9)
> Irakli, just to have a confirm: those pull request are stand alone, right? I
> can review them separately and they're not dependent by each other (at
> least, if I start from the lowest pull's number)?

There are dependencies but, I tried to note them in the pull requests themself.

Comment 11

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/677

Pointer to Github pull-request

Comment 12

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/678

Pointer to Github pull-request

Comment 13

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

ZER0 I think it would make most sense to do the reviews in the following order:

1. https://github.com/mozilla/addon-sdk/pull/668
2. https://github.com/mozilla/addon-sdk/pull/671
3. https://github.com/mozilla/addon-sdk/pull/677
4. https://github.com/mozilla/addon-sdk/pull/673
5. https://github.com/mozilla/addon-sdk/pull/672
6. https://github.com/mozilla/addon-sdk/pull/674
7. https://github.com/mozilla/addon-sdk/pull/678

Comment 14

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/234a1426c25fa3bc3d8d5ccd0ed43b94cf5b1151
Merge pull request #668 from Gozala/cleanup/hidden-frame

fix Bug 724632 - Rewrite hidden-frame in more conventional style r=@erikvold

Comment 15

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/5faedeb3ad37e1beb24b8227a8de66bb99a59588
Merge pull request #671 from Gozala/jep/disposable

Bug 724632 - Implementation of JEP-Disposable. r=@erikvold

Comment 16

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/b03a90ecadefd2ab49e2998383a7369f27cf2b83
Merge pull request #677 from Gozala/cleanup/class-fields@724632

Bug 724632 - Make data properties overridable on the instances. r=@erikvold

Comment 17

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

Irakli, you said to me to pause those reviews because the pull 692 (reducers); I'm waiting for your result of the meeting with Joe as you wrote in the github's comment. But let me know if we want to land these changes anyway or not.

Comment 18

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

Irakli, Mossop asked to me to remove the review's flag until you "unfroze" this reviews and I can work on it. Feel free add me again if/when the code will be ready to be reviewed!

Comment 19

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

Yeap that makes sense.

Comment 22

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1016

Pointer to Github pull-request

Comment 23

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/a321d01adb75f637e8902b43b0c8db2961880fc1
Merge pull request #1016 from Gozala/bug/content-events@724632

Bug 724632 - Implement content/chrome events. r=@jsantell

Comment 24

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

Attachment: Remove traits from window,tab,keyboard observers

Comment 25

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

Attachment: Detratify sdk/content/loader

Comment 26

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

Attachment: Implement helper module to never have to bind methods again

Comment 27

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

Attachment: Detratify sdk/deprecated/symbiont

Comment 28

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/76fcb01dec51dc170aaf85aba79d9126dd8f05a3
Merge pull request #1724 from Gozala/detraitify-observers

Bug 724632 - Remove traits from observers. r=@erikvold

Comment 29

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/481192bf13b7eac1596a378d8b407dc9265ec18a
Merge pull request #1733 from Gozala/detraitify-bond

Bug 724632 - Implement helper module to never have to bind methods again. r=@erikvold

Comment 30

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/b252a7be47cc9460f0218ad96170619e2b011278
Merge pull request #1725 from Gozala/detraitify-content-loader

Bug 724632 - Detraitify sdk/content/loader r=erikvold

Comment 31

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to [github robot] from comment #30)
> Commit pushed to master at https://github.com/mozilla/addon-sdk
> 
> https://github.com/mozilla/addon-sdk/commit/
> b252a7be47cc9460f0218ad96170619e2b011278
> Merge pull request #1725 from Gozala/detraitify-content-loader
> 
> Bug 724632 - Detraitify sdk/content/loader r=erikvold

Reverting this one, it caused a failure https://api.travis-ci.org/jobs/44399978/log.txt?deansi=true

Reverted this commit in https://github.com/mozilla/addon-sdk/commit/ceba54b1745e0ddf87bbc08ff3dc3c48c1698225

Comment 32

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8526323 [details] [review]
Detratify sdk/content/loader

Irakli, you'll have to take a look at this one when you get a chance.

Comment 33

[Erik Vold [:erikvold] (please needinfo? me)]

I've done the last patch to complete this bug in bug 1158534