Enable ESLint for devtools/client/shared/**/*.jsm

RESOLVED FIXED in Firefox 61

Status

enhancement
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

(Blocks 1 bug)

Trunk
Firefox 61
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

a year ago
These files are currently ignored for ESLint.

Similar to bug 1451659, we should work on enabling at least some coverage, so that we have basic syntax and various rule checks.

Rules that aren't quick-fixes or easy to do can be done in follow-up bugs, thanks to being able to selectively apply rules via glob-based configurations.
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8966147 [details]
Bug 1452575 - Automatically fix ESLint issues in shared jsm files in devtools.

https://reviewboard.mozilla.org/r/234880/#review240624

Looks great, thanks for working on it! :)

::: devtools/client/shared/DOMHelpers.jsm:90
(Diff revision 1)
>        }
>      }
>  
>      let child = null;
> -    if (previousSibling)  // then we are walking
> +    if (previousSibling) // then we are walking
> +      {

This brace should be on the previous line before the comment.
Attachment #8966147 - Flags: review?(jryans) → review+

Comment 4

a year ago
mozreview-review
Comment on attachment 8966148 [details]
Bug 1452575 - Enable ESLint for devtools/client/shared/**/*.jsm.

https://reviewboard.mozilla.org/r/234882/#review240626

Looks good, thanks! :)

::: .eslintignore:109
(Diff revision 1)
>  devtools/client/memory/test/chrome/*.html
>  devtools/client/performance/components/test/test_jit_optimizations_01.html
>  devtools/client/responsive.html/test/browser/touch.html
> -devtools/client/shared/*.jsm
> -devtools/client/shared/components/reps/reps.js
>  devtools/client/shared/components/reps/test/mochitest/*.html

I would suggest moving these other reps lines down to the imported section you adjusted.
Attachment #8966148 - Flags: review?(jryans) → review+
Assignee

Comment 5

a year ago
mozreview-review-reply
Comment on attachment 8966147 [details]
Bug 1452575 - Automatically fix ESLint issues in shared jsm files in devtools.

https://reviewboard.mozilla.org/r/234880/#review240624

> This brace should be on the previous line before the comment.

The second patch fixes all of these (as there's an ESLint rule for devtools already).
Assignee

Comment 6

a year ago
mozreview-review-reply
Comment on attachment 8966148 [details]
Bug 1452575 - Enable ESLint for devtools/client/shared/**/*.jsm.

https://reviewboard.mozilla.org/r/234882/#review240626

> I would suggest moving these other reps lines down to the imported section you adjusted.

Thank you for spotting that, I'd missed noticing those. Looking at mozilla-central, there's now no devtools/client/shared/components/reps/test/ directory, and the html files referenced aren't anywhere to be seen, so I'll just remove those lines completely.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 9

a year ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4ed9006a80a0
Automatically fix ESLint issues in shared jsm files in devtools. r=jryans
https://hg.mozilla.org/integration/autoland/rev/8ef9106d1ae1
Enable ESLint for devtools/client/shared/**/*.jsm. r=jryans

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4ed9006a80a0
https://hg.mozilla.org/mozilla-central/rev/8ef9106d1ae1
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61

Updated

a year ago
Duplicate of this bug: 1275637

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.