Closed Bug 1395903 Opened 2 years ago Closed 2 years ago

Remove lazy declaration of sdk/core/heritage for "Class" in devtools/server/performance/recorder.js

Categories

(DevTools :: General, enhancement, P1)

enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: sole, Assigned: sole)

References

Details

(Whiteboard: [nosdk])

Attachments

(1 file)

The sdk/core/heritage file is loaded lazily here
http://searchfox.org/mozilla-central/source/devtools/server/performance/recorder.js#11

We mentioned it in https://bugzilla.mozilla.org/show_bug.cgi?id=1378856#c1 but no bug was filed so it escaped us!

We need to use ES6 native classes instead of the SDK's Class helper, but I do not even see this being used in the file.
Assignee: nobody → spenades
(In reply to Soledad Penades [:sole] [:spenades] from comment #0)

> We mentioned it in https://bugzilla.mozilla.org/show_bug.cgi?id=1378856#c1
> but no bug was filed so it escaped us!

It didn't escape us because no bug was filed, bug 1378856 was exactly for that. :) But it escape to Honza my comment and I escape to me when I did the review, it's my fault. This bug is a follow up to bug 1378856 for removing the lazy loading declaration.

> We need to use ES6 native classes instead of the SDK's Class helper, but I
> do not even see this being used in the file.

That's because it uses decorators, that cannot be used in ES6 classes (see reviews comment of bug 1378856, and https://github.com/devtools-html/snippets-for-removing-the-sdk#how-to-deal-with-decorators).
See Also: → 1378856
Summary: Stop using sdk/core/heritage in DevTools performance server recorder.js → Remove lazy declaration of sdk/core/heritage for "Class" in devtools/server/performance/recorder.js
Blocks: 1368939
No longer blocks: 1350645
I don't really know why you changed the bug title, Matteo. Not sure what that adds to the bug.

Also, on further examination, the code is not using sdk/core/heritage AKA `Class` simply because Jan updated it on the previous-previous commit to not use `Class`: https://hg.mozilla.org/mozilla-central/diff/bf2a7774f7b9/devtools/server/performance/recorder.js#l1.12

There's no need to bring up the decorators story.

Anyway here is a nicely green try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ebf2fb319e9d2baa90b69785a6aabc9714bbd9a
Comment on attachment 8903561 [details]
Bug 1395903 - Stop using sdk/core/heritage in devtools performance server recorder.js.

https://reviewboard.mozilla.org/r/175374/#review180468

Looks good, and good catch :)
Attachment #8903561 - Flags: review?(jdescottes) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8702d05b5df7
Stop using sdk/core/heritage in devtools performance server recorder.js.r=jdescottes
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8702d05b5df7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Flags: qe-verify-
Priority: -- → P1
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.