Closed Bug 1309796 Opened 9 years ago Closed 9 years ago

add eslint support for netmonitor

Categories

(DevTools :: Netmonitor, defect, P1)

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Iteration:
52.2 - Oct 17
Tracking Status
firefox52 --- fixed

People

(Reporter: gasolin, Assigned: gasolin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [netmonitor])

Attachments

(1 file)

netmonitor/test/ has eslint support, but netmonitor/ does not. Expect: Add .eslintrc and fix related lint error in netmonitor/ repo
Flags: qe-verify?
It's an engineering only issue
Assignee: nobody → gasolin
Flags: qe-verify? → qe-verify-
Status: NEW → ASSIGNED
When I append: `var notUsedVar;` into netmonitor/panel.js, and run: `./mach eslint devtools/client/netmonitor` I am getting an error as expected: Z 12:5 error 'notUsedVar' is defined but never used. no-unused-vars (eslint) ✖ 1 problem (1 error, 0 warnings) What errors in `netmonitor/` dir do you see? Honza
As inspector/.eslintrc , we can identify some chrome privilege requires and comment them in source https://github.com/mozilla/gecko-dev/blob/7e0678438a65e310f2392ca79b77cba6ec08f7f4/devtools/client/inspector/.eslintrc
So, this bug is about disallowing requiring chrome code, correct? Honza
Flags: needinfo?(gasolin)
Just want netmonitor have same configurations as inspector. It's not actually disallowing require chrome code, but only fix warnings by addressing them with disable flags.
Flags: needinfo?(gasolin)
Comment on attachment 8800568 [details] Bug 1309796 - add eslint support for netmonitor; https://reviewboard.mozilla.org/r/85484/#review84132 Looks good to me, and Eslint is green for me. R+, assuming my comment is resolved. Thanks! Honza ::: devtools/client/netmonitor/.eslintrc:10 (Diff revision 1) > + "rules": { > + // The inspector is being migrated to HTML and cleaned of > + // chrome-privileged code, so this rule disallows requiring chrome > + // code. Some files in the inspector disable this rule still. The > + // goal is to enable the rule globally on all files. > + "mozilla/reject-some-requires": [2, "^(chrome|chrome:.*|resource:.*|devtools/server/.*|.*\\.jsm|devtools/shared/platform/(chome|content)/.*)$"], Please update the comment, it's used for the Network panel now (not the Inspector).
Attachment #8800568 - Flags: review?(odvarko) → review+
comment updated, thanks!
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b2f3dd7eec5d add eslint support for netmonitor; r=Honza
Keywords: checkin-needed
Priority: -- → P1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Iteration: --- → 52.2 - Oct 17
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: