1309796 - add eslint support for netmonitor

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[[e-mail]]

netmonitor/test/ has eslint support, but netmonitor/ does not.

Expect:

Add .eslintrc and fix related lint error in netmonitor/ repo

Comment 1

[[e-mail]]

It's an engineering only issue

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 3

[[e-mail]]

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

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

So, this bug is about disallowing requiring chrome code, correct?

Honza

Comment 5

[[e-mail]]

Attachment: Bug 1309796 - add eslint support for netmonitor;

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

Comment 6

[[e-mail]]

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.

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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).

Comment 8

[[e-mail]]

Comment on attachment 8800568 [details]
Bug 1309796 - add eslint support for netmonitor;

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

Comment 9

[[e-mail]]

comment updated, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2f3dd7eec5d
add eslint support for netmonitor; r=Honza

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b2f3dd7eec5d