Closed Bug 1393497 Opened 2 years ago Closed 2 years ago
[nosdk] Valence addon still using sdk dependencies
See https://github.com/mozilla/valence/search?utf8=%E2%9C%93&q=sdk&type= We need to update the Valence addon to no longer rely on the SDK classes.
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [reserve-nosdk] → [nosdk]
Target Milestone: --- → Firefox 57
I double checked that Valence is not shipped with DevEdition; but it's just installed the first time WebIDE is opened in any Firefox versions, despite what the Valence's repo said. It seems to me, that the quickest way to avoid that, it's just set this pref to `false`: http://searchfox.org/mozilla-central/source/devtools/client/webide/webide-prefs.js#7 Or remove the pref, and hard code this to `false`: http://searchfox.org/mozilla-central/source/devtools/client/webide/content/webide.js#93 Since AFAIK we want to remove WebIDE code anyway, it doesn't seems to me valuable refactoring WebIDE code to remove any Valence / Adapters reference. Alex, since you've more insight about adb-helper / valence than me, what do you think? Seems reasonable proceed in this way?
Removing the pref and all code that use it sounds like a good approach. But that's not all the entry points for Valence. There is also an entry for it in "extra components" panel, in WebIDE select "Project > Manage extra components" menu entry, you will see "Tools adapters Add-on" entry. This should be implemented here: http://searchfox.org/mozilla-central/source/devtools/client/webide/content/addons.js#85-94 Otherwise a code search on "adapters" should help identifying all things related to valence: http://searchfox.org/mozilla-central/search?q=adapters&case=false®exp=false&path=devtools%2Fclient%2Fwebide Also we may put some messages on MDN to say it is no longer maintained: https://developer.mozilla.org/en-US/docs/Tools/WebIDE/Troubleshooting#Connecting_to_other_browsers_(Chrome_Safari_iOS) https://developer.mozilla.org/en-US/docs/Tools/Valence
Alex, I filed bug 1397274 for the docs, thanks! About the patch, I run the webIDE tests (devtools/client/webide/test/) and I got a couple of errors, especially one modal saying: "Can’t fetch the add-on list: JSON not available, CDN error: [object ProgressEvent], storage error: Empty cache for devtools.webide.addonsURL" but since I got the same results in mozilla central without the patch updated, I believe is not related to this changes. Build and run WebIDE seems fine, but I don't know the codebase, so let's double check to see if I missed anything else.
Comment on attachment 8905303 [details] Bug 1393497 - remove Valence from WebIDE; https://reviewboard.mozilla.org/r/177100/#review182712 Thanks Matteo for this cleanup, it looks good. r+ assuming you get a green try.
Attachment #8905303 - Flags: review?(poirot.alex) → review+
Here the try build, seems the oranges are not related to this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=149c12652e96b7ed288378c862e3318c86cea3b8&selectedJob=129530737
Triggered Autoland from Review Board.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/9815926c3bc1 remove Valence from WebIDE; r=ochameau
You need to log in before you can comment on or make changes to this bug.