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

RESOLVED FIXED in Firefox 55

Status

enhancement
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: standard8, Assigned: rajk, Mentored)

Tracking

3 Branch
mozilla55

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [lang=js])

Attachments

(1 attachment)

Reporter

Description

2 years ago
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
Assignee

Comment 1

2 years ago
can i take this one?
Reporter

Comment 2

2 years ago
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.
Assignee: nobody → rajesh.kathiriya507
Comment hidden (mozreview-request)
Reporter

Comment 4

2 years ago
mozreview-review
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.
Attachment #8866892 - Flags: review?(standard8)
Comment hidden (mozreview-request)
Reporter

Comment 6

2 years ago
mozreview-review
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.
Attachment #8866892 - Flags: review?(standard8) → review+
Comment hidden (mozreview-request)

Comment 8

2 years ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/de31a15ecd05
Added Eslint file for the eslint-plugin-mozilla configs r=standard8

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/de31a15ecd05
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

Last year
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.