1549166 - Turn on ESLint in all of comm-central

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

I'm going to use this bug for a few clean-up tasks remaining. All that's left to be linted is mailnews (bug 1515877), ldap, and mail/test/mozmill.

After that I'm going to clean up the .eslintignore file and adjust the top config file to eliminate all the identical config files I've left in test directories everywhere.

Comment 1

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: 1549166-eslint-ldap-1.diff

This is the most difficult directory.

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 9062775 [details] [diff] [review]
1549166-eslint-ldap-1.diff

Review of attachment 9062775 [details] [diff] [review]:
-----------------------------------------------------------------

I am in fact *the best* reviewer for this since I used the test as basis for https://searchfox.org/comm-central/source/mailnews/local/test/unit/test_mailboxURL.js.

::: ldap/xpcom/tests/unit/test_nsLDAPURL.js
@@ +2,5 @@
>  /*
>   * Test suite for nsLDAPURL functions.
>   */
>  
> +var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");

You need this? The mailbox URL test doesn't have that import.

Comment 3

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

You need this? The mailbox URL test doesn't have that import.

It has a head file which imports Services.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e344d6227471
Turn on ESLint in ldap; r=jorgk

Comment 5

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: 1549166-eslint-config-1.diff

This is a copy of the patch in bug 1415265.

Comment 6

[:aceman]

Comment on attachment 9064952 [details] [diff] [review]
1549166-eslint-config-1.diff

Review of attachment 9064952 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

::: .eslintrc.js
@@ +80,5 @@
> +  }, {
> +    // If it is an xpcshell head file, we turn off global unused variable checks, as it
> +    // would require searching the other test files to know if they are used or not.
> +    // This would be expensive and slow, and it isn't worth it for head files.
> +    // We could get developers to declare as exported, but that doesn't seem worth it.

Is this missing some word?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/13665dfc61d8
Centralise configuration of ESLint environments for test directories; r=aceman

Comment 8

[Patrick Cloke [:clokep]]

Attachment: Use centralized configs for chat

Chat tests use slightly different paths, so this adds another path entry to the root .estlintrc.js and deletes a bunch more .eslintrc.js files from chat.

Comment 9

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Comment on attachment 9066099 [details] [diff] [review]
Use centralized configs for chat

Nice one, thanks Patrick. I wanted to remove these files but didn't have a reasonable way to do it. This is as good as anything I came up with.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9efee8d57a4c
Use centralized ESLint configuration for chat tests. r=darktrojan

Comment 11

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/comm-beta/rev/f86e8e7e79023267bbf1b1e7e6db9c21ca3f5a4d

The last patch joined its friends on beta, also to trigger a build which didn't happen after the merge.

Comment 12

[:aceman]

Just to make it clear, the bug summary mentions all of comm-central, but in none of the bugs I have seen changes to Seamonkey (/suite subfolder). So it should probably be stated that that was an explicit non-goal of this cleanup and /suite is NOT being eslinted.

Comment 13

[Patrick Cloke [:clokep]]

(In reply to Geoff Lankow (:darktrojan) from comment #9)

Nice one, thanks Patrick. I wanted to remove these files but didn't have a
reasonable way to do it. This is as good as anything I came up with.

It isn't perfect, but it was the best I could come up with! Thanks for the review (and thanks for checking in / uplifting Jorg!)