1378862 - Stop using sdk/core/heritage in DevTools shared protocol.js

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P1)

Description

[Soledad Penades [:sole] [:spenades]]

Used in: devtools/shared/protocol.js

More details to follow as we triage.

Comment 1

[Soledad Penades [:sole] [:spenades]]

Some of the classes in the file extend EventTarget, so can't reliably replace Class with ES6 class without possible issues here until we deal with the EventTarget bit. Blocked by bug 1381542 then :)

Comment 2

[Julian Descottes [:jdescottes]]

Repurposing Bug 1137935 to remove all usage of sdk/* in shared/protocol.js

Comment 3

[Julian Descottes [:jdescottes]]

Sorry about that but Bug 1137935 ended up taking a slightly different direction and this still needs to be addressed.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1378862 - remove usage of sdk heritage in devtools protocol.js;

Review commit: https://reviewboard.mozilla.org/r/170868/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170868/

Comment 5

[Julian Descottes [:jdescottes]]

This is a very conservative patch, that allows to get rid of the SDK APIs but should have several followups for cleanup and improvements.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4e6f5c291677fcf5feca1110c6e918b394a951e

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8899568 [details]
Bug 1378862 - remove usage of sdk heritage in devtools protocol.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170868/diff/1-2/

Comment 7

[Julian Descottes [:jdescottes]]

Here is the list of followups that should be filed if we go for this:
- always call Arg, Option and RetVal with the new keyword
- migrate Arg, Option and RetVal to ES6 classes
- use constructor instead of initialize in actors and fronts that inherit Actor/Front classes
- migrate Pool, Actor and Front to ES6 classes

Comment 8

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

Comment on attachment 8899568 [details]
Bug 1378862 - remove usage of sdk heritage in devtools protocol.js;

https://reviewboard.mozilla.org/r/170868/#review177408

If we're going to have follow up bugs as we discussed, I don't have anything to complain or add to this, it looks good to me; so if tests are passing, let's ship it.

Comment 9

[Julian Descottes [:jdescottes]]

Comment on attachment 8899568 [details]
Bug 1378862 - remove usage of sdk heritage in devtools protocol.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170868/diff/2-3/

Comment 10

[Julian Descottes [:jdescottes]]

Thanks for the review! Just pushed to try and rebased on the latest central: https://treeherder.mozilla.org/#/jobs?repo=try&revision=da352b5ba4044eb37d34106074d4982c7b45b679

Will log the follow ups while waiting for the results.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/115d4b8f1f0a
remove usage of sdk heritage in devtools protocol.js;r=zer0

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/115d4b8f1f0a