1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Mark Banner (:standard8)]

There's a couple of things wrong with the current handling for workers:

- It doesn't correctly detect globals from modules
- It doesn't have an option if a worker doesn't have 'worker' in its filename.

In the patch I'm addressing, I'm also dealing with some files imported into workers as modules.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

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

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

https://reviewboard.mozilla.org/r/111292/#review113510

Rather than using /* eslint-is-worker */ can we just create our own environment for workers then use /* eslint-env worker */ ?
http://eslint.org/docs/developer-guide/working-with-plugins#environments-in-plugins

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

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

Comment 4

[Mark Banner (:standard8)]

I hadn't noticed the environments option, but that's definitely a lot better. I also discovered there's already a "worker" environment, so extending that for chrome workers seems the good option.

Comment 5

[Dave Townsend [:mossop]]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

https://reviewboard.mozilla.org/r/111292/#review113876

::: tools/lint/eslint/eslint-plugin-mozilla/package.json:3
(Diff revision 2)
>  {
>    "name": "eslint-plugin-mozilla",
>    "version": "0.2.20",

Do we still need to bump the version number here?

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

https://reviewboard.mozilla.org/r/111292/#review113880

::: tools/lint/eslint/eslint-plugin-mozilla/package.json:3
(Diff revision 2)
>  {
>    "name": "eslint-plugin-mozilla",
>    "version": "0.2.20",

Yes, please bump the version to force local users to update on next run.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

https://reviewboard.mozilla.org/r/111292/#review113876

> Do we still need to bump the version number here?

Yes, that got lost in a rebase unfortunately. I'll add it back in.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03bfb1ccd33f
Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports. r=mossop

Comment 10

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=77590058&repo=autoland

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dee6c94829e4
Backed out changeset 03bfb1ccd33f for test failures in test_loading.xul

Comment 12

[Mark Banner (:standard8)]

The test failures were due to the test expecting failures on specific line numbers, adding an eslint comment affected those.

Hence I'm updating the test for the new line numbers, and adding a comment to warn others.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8835617 [details]
Bug 1338249 - Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/111292/diff/3-4/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49c83f6481dc
Improve eslint-plugin-mozilla's handling of workers when dealing with globals and imports. r=mossop

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/49c83f6481dc