Closed Bug 1451659 Opened 3 years ago Closed 3 years ago

Enable ESLint for devtools/client/{canvasdebugger,framework,scratchpad,shadereditor}

Categories

(DevTools :: General, enhancement)

Product:
DevTools
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(7 files)

Bug 1451659 - Remove obsolete import-globals-from definitions that also stop ESLint from running in devtools.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1451659 - Automated ESLint fixes for devtools/client/framework/.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1451659 - Automated ESLint fixes for devtools/client/canvasdebugger/.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1451659 - Automated ESLint fixes for devtools/client/scratchpad/.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1451659 - Automated ESLint fixes for devtools/client/shadereditor/.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1451659 - Enable ESLint for devtools/client/{canvasdebugger,framework,scratchpad,shadereditor}/.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1451659 - Fix mochitest issues after enabling ESLint on more files in devtools.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details 
These directories are currently ignored for ESLint.

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.
Julian, feel free to reassign reviews to better people if necessary.

The devtools/.eslintrc.js overrides I've tried to do in such a way that we can make it easy for contributors to pick up mentored bugs without clashing too much in terms of conflicts. I've also tried to keep the amount we're disabling rules on to a minimum.
Comment on attachment 8965273 [details]
Bug 1451659 - Automated ESLint fixes for devtools/client/framework/.

https://reviewboard.mozilla.org/r/233998/#review239630
Attachment #8965273 - Flags: review?(jdescottes) → review+
Comment on attachment 8965274 [details]
Bug 1451659 - Automated ESLint fixes for devtools/client/canvasdebugger/.

https://reviewboard.mozilla.org/r/234000/#review239632
Attachment #8965274 - Flags: review?(jdescottes) → review+
Comment on attachment 8965275 [details]
Bug 1451659 - Automated ESLint fixes for devtools/client/scratchpad/.

https://reviewboard.mozilla.org/r/234002/#review239636
Attachment #8965275 - Flags: review?(jdescottes) → review+
Comment on attachment 8965276 [details]
Bug 1451659 - Automated ESLint fixes for devtools/client/shadereditor/.

https://reviewboard.mozilla.org/r/234004/#review239638
Attachment #8965276 - Flags: review?(jdescottes) → review+
Comment on attachment 8965277 [details]
Bug 1451659 - Enable ESLint for devtools/client/{canvasdebugger,framework,scratchpad,shadereditor}/.

https://reviewboard.mozilla.org/r/234006/#review239646

::: .eslintignore:168
(Diff revision 1)
>  devtools/shared/pretty-fast/*
>  devtools/shared/sourcemap/*
>  devtools/shared/sprintfjs/*
>  devtools/shared/qrcode/decoder/*
>  devtools/shared/qrcode/encoder/*
> +devtools/client/debugger/new/**

Already at L152?

::: devtools/.eslintrc.js:112
(Diff revision 1)
> +  }, {
> +    "files": [
> +      // Note: Bug 1403938 may be removing canvasdebugger, check before
> +      // doing more work on enabling these rules.
> +      "client/canvasdebugger/**",
> +      // Note: Bug 1342237 may be removing canvasdebugger, check before

nit: canvasdebugger -> shadereditor
Attachment #8965277 - Flags: review?(jdescottes) → review+
Comment on attachment 8965278 [details]
Bug 1451659 - Fix mochitest issues after enabling ESLint on more files in devtools.

https://reviewboard.mozilla.org/r/234008/#review239648
Attachment #8965278 - Flags: review?(jdescottes) → review+
Comment on attachment 8965272 [details]
Bug 1451659 - Remove obsolete import-globals-from definitions that also stop ESLint from running in devtools.

https://reviewboard.mozilla.org/r/233996/#review239650

LGTM! I quickly browsed the non-whitespace changes, didn't see anything fishy that wasn't fixed by the last changeset.
Thanks.
Comment on attachment 8965272 [details]
Bug 1451659 - Remove obsolete import-globals-from definitions that also stop ESLint from running in devtools.

https://reviewboard.mozilla.org/r/233996/#review239658
Attachment #8965272 - Flags: review?(jdescottes) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c17e4c4c952
Remove obsolete import-globals-from definitions that also stop ESLint from running in devtools. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/09273b8f1e61
Automated ESLint fixes for devtools/client/framework/. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/0a70c315cf90
Automated ESLint fixes for devtools/client/canvasdebugger/. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/2ac7c4d5d184
Automated ESLint fixes for devtools/client/scratchpad/. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/61316c057950
Automated ESLint fixes for devtools/client/shadereditor/. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/a45a86e9a6e3
Enable ESLint for devtools/client/{canvasdebugger,framework,scratchpad,shadereditor}/. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/e20559cdf1c4
Fix mochitest issues after enabling ESLint on more files in devtools. r=jdescottes
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.