1359019 - Enable sort-keys ESLint rule for the eslint-plugin-mozilla configs

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Mark Banner (:standard8)]

To make it easier to find rules and globals in our main configs, I think we should enforce that the object keys for those files are in alphabetical order.

This will make it easier for updating those files, and save review comments.

I'm happy to mentor this.

The relevant directory is:

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/

You'll need to add a minimal .eslintrc.js file which specifies the rule:

"sort-key": "error"

and then fix the issues raised by ESLint for the files in that directory.

More details on ESlint and how to run it here: https://developer.mozilla.org/docs/ESLint

Comment 1

[Rajesh Kathiriya(:rajk)]

can i take this one?

Comment 2

[Mark Banner (:standard8)]

Sure, though you might want to wait for bug 1358947 / bug 1358949 to merge to mozilla-central, if you're basing directly off of m-c, as they both change the recommended.js file.

The merge should happen later today for those.

Comment 3

[Rajesh Kathiriya(:rajk)]

Attachment: bug 1359019 - Added Eslint file for the eslint-plugin-mozilla configs

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

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8866892 [details]
bug 1359019 - Added Eslint file for the eslint-plugin-mozilla configs

https://reviewboard.mozilla.org/r/138502/#review141994

::: tools/lint/eslint/eslint-plugin-mozilla/lib/configs/.eslintrc.js:5
(Diff revision 1)
> +"use strict";
> +
> +module.exports = {
> +    // Require object keys to be sorted. 
> +    "sort-key": "error"

This doesn't actually work at the moment (it should be raising errors). Partially my fault as it looks like I did a typo in the rule name.

The module.exports section needs to be:

```
module.exports = {
  rules: {
    // Require object keys to be sorted.
    "sort-keys": "error"
  }
};
```

Please also drop the whitespace at the end of the comment.

Once you've fixed that, there should be a number of errors in the config/*.js files to fix.

Comment 5

[Rajesh Kathiriya(:rajk)]

Comment on attachment 8866892 [details]
bug 1359019 - Added Eslint file for the eslint-plugin-mozilla configs

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

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8866892 [details]
bug 1359019 - Added Eslint file for the eslint-plugin-mozilla configs

https://reviewboard.mozilla.org/r/138502/#review142462

Thank you for the updates, much nicer! One slight indentation fix to the .eslintrc.js file and we can then land this.

::: tools/lint/eslint/eslint-plugin-mozilla/lib/configs/.eslintrc.js:4
(Diff revision 2)
> +"use strict";
> +
> +module.exports = {
> +    rules: {

Sorry I didn't notice it before, please could you change this file to two-space indent.

Comment 7

[Rajesh Kathiriya(:rajk)]

Comment on attachment 8866892 [details]
bug 1359019 - Added Eslint file for the eslint-plugin-mozilla configs

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de31a15ecd05
Added Eslint file for the eslint-plugin-mozilla configs r=standard8

Comment 9

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

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