936623 - Deprecate widget

Status: RESOLVED FIXED

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

Description

[Dave Townsend [:mossop]]

When australis finally ships we should deprecate widget and add warnings when it is used.

Comment 1

[Dave Townsend [:mossop]]

We also need to update the docs to note the deprecation.

Comment 2

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

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

Pointer to Github pull-request

Comment 3

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

The deprecation process assumes we deprecate a module only when we provide alternatives, so I would wait to land this bug till Toolbar, Frame, and the new Buttons are landed.

Dave, is this bug intended to update the documentation as well, as it seems by your comment, or should be done in a different bug?

Comment 4

[Dave Townsend [:mossop]]

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #3)
> The deprecation process assumes we deprecate a module only when we provide
> alternatives, so I would wait to land this bug till Toolbar, Frame, and the
> new Buttons are landed.

Yep, agree with this, but they should all be landing imminently, right?

> Dave, is this bug intended to update the documentation as well, as it seems
> by your comment, or should be done in a different bug?

It doesn't really matter to me, if you want a second bug for the docs then file one.

Comment 5

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

They should all be landing these days. However, I will wait to land this patch until we didn't land the rest – that's why I set the two related bugs as blockers.

About the documentation I do not want a second bug, but because the patch doesn't provide any changes on documentation I thought it should be added.

Comment 6

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

Changed my mind, now that documentation is in MDN we shouldn't block PR for that – 'cause documentation can't be added in the same PR via MD files.

Comment 7

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

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

Now that we have both Button and Toolbar API we can land this bug.

Comment 8

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/be9558af8c3b23f19950b1fd0dd49d8854e9cef3
Bug 936623 - Deprecate Widget

https://github.com/mozilla/addon-sdk/commit/d59daa166d5da3569f92332a8a6182fe6eeaf818
Merge pull request #1326 from erikvold/936623

Bug 936623 - Deprecate Widget r=@ZER0

Comment 9

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1341/files#diff-4

Pointer to Github pull-request

Comment 10

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

Comment on attachment 8358734 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1341/files#diff-4

Actually I'll give this to Matteo.

Comment 11

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

Comment on attachment 8358734 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1341/files#diff-4

review rejected basically because it seems to me that removes all the code that checks if we currently run Australis or not, and that will broke Holly.

Comment 12

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

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #11)
> Comment on attachment 8358734 [details]
> Pointer to Github pull request:
> https://github.com/mozilla/addon-sdk/pull/1341/files#diff-4
> 
> review rejected basically because it seems to me that removes all the code
> that checks if we currently run Australis or not, and that will broke Holly.

As far as I understood our last weekly meeting, we agreed to not land this warning until Australis lands, so does it make sense to change the patch?

Comment 13

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

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #12)
> (In reply to Matteo Ferretti [:matteo] [:zer0] from comment #11)
> > Comment on attachment 8358734 [details]
> > Pointer to Github pull request:
> > https://github.com/mozilla/addon-sdk/pull/1341/files#diff-4
> > 
> > review rejected basically because it seems to me that removes all the code
> > that checks if we currently run Australis or not, and that will broke Holly.
> 
> As far as I understood our last weekly meeting, we agreed to not land this
> warning until Australis lands, so does it make sense to change the patch?

So, during the last meeting we thought Australis was going to land on 30, but they changed their plans and they're trying to land on 29. It means that basically nothing is changed: we still need to support Holly (so, the code that checks if we're in australis or not needs to be present) and we have to land deprecation widget messages.

Comment 14

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

Attachment: https://github.com/mozilla/addon-sdk/pull/1436/files

Comment 15

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

Comment on attachment 8393013 [details]
https://github.com/mozilla/addon-sdk/pull/1436/files

r+ once the conflict is resolved and all tests are passing

Comment 16

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

Comment on attachment 8393013 [details]
https://github.com/mozilla/addon-sdk/pull/1436/files

Hey I've got to get another review, there are updates due to failures.

Comment 17

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

Also the test examples fail, maybe we can make handling those a separate bug?  I suggest we remove them, rewrite them to use ui/stuff, or move them to a separate repo that will use jpm to run those tests (like how mozmill used to work.

Comment 18

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

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #17)
> Also the test examples fail, maybe we can make handling those a separate
> bug?

Sounds good to me.

> I suggest we remove them, rewrite them to use ui/stuff, or move them
> to a separate repo that will use jpm to run those tests (like how mozmill
> used to work.

Because are examples, probably is better update them to use new APIs.

Also, it seems to me that most of our examples doesn't have actually any tests (annotator, etc), and the one that have them – and doesn't work because the deprecation message – are really dummy tests. I'm totally in favor to remove them, and fill a bug to update our examples' code, and examples' test.
We could also fill multiple bugs (one per example), they could be good first bug too, and help contributors to get familiarity with new APIs.

However, r+ once you removed the test examples that are failing, and maybe add a comment in the tests file that points to the bug you filed.

Comment 19

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/f937e881f5104527e1587c4f9d07ded2b4be590a
Bug 936623 - Deprecate Widget

https://github.com/mozilla/addon-sdk/commit/7b75b2ae4ef68cf6b13f3be1aa95fcd434119f68
Bug 936623 fixing a 280 of 281 failure for test-widget.js due to the added deprecation warning

https://github.com/mozilla/addon-sdk/commit/5298fdb5f58cf81a006d1f4df9b57a45779e9a70
Bug 936623 commenting out the broken example add-on tests (due to widget dep)

https://github.com/mozilla/addon-sdk/commit/313abba9bdd6523c2b716d44b806f53cfb89b980
Merge pull request #1436 from erikvold/936623

Bug 936623 - Deprecate Widget r=@ZER0