Closed Bug 1325191 Opened 8 years ago Closed 8 years ago

Fix Eslint errors in devtools/server/*.js

Categories

(DevTools :: Shared Components, defect)

Product:
DevTools
Component:
Shared Components
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(2 files)

Bug 1325191 - Fix Eslint errors in devtools/server/*.js.
59 bytes, text/x-review-board-request
jryans
: review+
Details
Bug 1325191 - Fix ESLint errors in devtools/server/performance/.
59 bytes, text/x-review-board-request
jryans
: review+
Details
No description provided.
Comment on attachment 8820881 [details] Bug 1325191 - Fix Eslint errors in devtools/server/*.js. https://reviewboard.mozilla.org/r/100266/#review100892 Thanks for working on this clean up! ::: devtools/server/actors/utils/TabSources.js:304 (Diff revision 5) > - } else if (aSource.introductionType === "wasm") { > + } else if (source.introductionType === "wasm") { > // Wasm sources are not JavaScript. Give them their own content-type. > spec.contentType = "text/wasm"; > - } else { > + } else if (url) { > - if (url) { > // There are a few special URLs that we know are JavaScript: Fix comment indentation ::: devtools/server/actors/utils/TabSources.js:317 (Diff revision 5) > - let pathname = new URL(url).pathname; > + let pathname = new URL(url).pathname; > - let filename = pathname.slice(pathname.lastIndexOf("/") + 1); > + let filename = pathname.slice(pathname.lastIndexOf("/") + 1); > - let index = filename.lastIndexOf("."); > + let index = filename.lastIndexOf("."); > - let extension = index >= 0 ? filename.slice(index + 1) : ""; > + let extension = index >= 0 ? filename.slice(index + 1) : ""; > - if (extension === "xml") { > + if (extension === "xml") { > // XUL inline scripts may not correctly have the Fix comment indentation ::: devtools/server/actors/utils/TabSources.js:325 (Diff revision 5) > - } > + } else if (extension === "js") { > - else if (extension === "js") { > - spec.contentType = "text/javascript"; > + spec.contentType = "text/javascript"; > - } > + } > - } catch (e) { > + } catch (e) { > // This only needs to be here because URL is not yet exposed to Fix comment indentation ::: devtools/server/actors/utils/TabSources.js:336 (Diff revision 5) > - } > + } > - } > + } > - } > + } > - } > + } else { > - else { > // Assume the content is javascript if there's no URL Fix comment indentation ::: devtools/server/actors/utils/TabSources.js:430 (Diff revision 5) > - return resolve(this._sourceMaps.get(aSource)); > + return resolve(this._sourceMaps.get(source)); > }, > > /** > * Set a SourceMapConsumer for the source map for > - * |aSource|. > + * |source|. Seems like this would fit on the previous line ::: devtools/server/actors/utils/map-uri-to-addon-id.js:28 (Diff revision 5) > * to worry about them. > */ > if (!Services.appinfo > || Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT > - || Services.appinfo.ID === undefined /* XPCShell */ > + > + /* XPCShell */ Mmm, I don't really like this ESLint rule that bans inline comments... To me, the previous version is more natural. What do you think? ::: devtools/server/actors/utils/webconsole-listeners.js:1 (Diff revision 5) > +"use strict"; Add MPL license header. ::: devtools/server/worker.js:51 (Diff revision 5) > switch (packet.type) { > case "connect": > // Step 3: Create a connection to the parent. > let connection = DebuggerServer.connectToParent(packet.id, this); > connections[packet.id] = { > - connection : connection, > + connection: connection, Just `connection,` on this line should work
Attachment #8820881 - Flags: review?(jryans) → review+
Comment on attachment 8820881 [details] Bug 1325191 - Fix Eslint errors in devtools/server/*.js. https://reviewboard.mozilla.org/r/100266/#review100894 ::: devtools/server/worker.js:48 (Diff revision 5) > > this.addEventListener("message", function (event) { > let packet = JSON.parse(event.data); > switch (packet.type) { > case "connect": > // Step 3: Create a connection to the parent. Seems like the comment indentation could be improved here
Comment on attachment 8820896 [details] Bug 1325191 - Fix ESLint errors in devtools/server/performance/. https://reviewboard.mozilla.org/r/100280/#review100896 Looks like a good clean up, thanks for working on it! ::: devtools/server/performance/memory.js:196 (Diff revision 3) > return this._getCurrentTime(); > } > > this._frameCache.initFrames(); > > - this.dbg.memory.allocationSamplingProbability = options.probability != null > + this.drainAllocationsTimeoutTimer = typeof drainAllocationsTimeout === "number" ? Maybe this `typeof` check isn't needed anymore? Seems like you default to null already, and it would strange for someone to pass a non-number... Seems like a language with types is a better fix than `typeof` for this. :) ::: devtools/server/performance/memory.js:196 (Diff revision 3) > return this._getCurrentTime(); > } > > this._frameCache.initFrames(); > > - this.dbg.memory.allocationSamplingProbability = options.probability != null > + this.drainAllocationsTimeoutTimer = typeof drainAllocationsTimeout === "number" ? Hmm, it looks like the new version doesn't use the `probability` arg? I would think it still needs to be set on `this.dbg.memory.allocationSamplingProbability`. ::: devtools/server/performance/profiler.js:242 (Diff revision 3) > isActive: function () { > let isActive = nsIProfilerModule.IsActive(); > let elapsedTime = isActive ? nsIProfilerModule.getElapsedTime() : undefined; > let { position, totalSize, generation } = this.getBufferInfo(); > - return { isActive: isActive, currentTime: elapsedTime, position, totalSize, generation }; > + return { > + isActive: isActive, Just `isActive,` should work ::: devtools/server/performance/profiler.js:566 (Diff revision 3) > * @param {function} handler > * @return {function} > */ > function sanitizeHandler(handler, identifier) { > return DevToolsUtils.makeInfallible(function (subject, topic, data) { > - subject = (subject && !Cu.isXrayWrapper(subject) && subject.wrappedJSObject) || subject; > + subject = (subject && !Cu.isXrayWrapper(subject) && subject.wrappedJSObject) Might be less mysterious as an if block instead? Up to you. Also, I would generally put the `||` on the previous line to clarify that the expression continues, but this case is really a bit unusual.
Attachment #8820896 - Flags: review?(jryans) → review+
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fe99e9ae7fce Fix Eslint errors in devtools/server/*.js. r=jryans https://hg.mozilla.org/integration/mozilla-inbound/rev/fd069b26630c Fix ESLint errors in devtools/server/performance/. r=jryans
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/88b62488ca30 Fix test_memory_allocations_* failures. r=bustage
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: