Closed Bug 1359019 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox Build System :: Lint and Formatting, enhancement)

3 Branch
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: standard8, Assigned: rajk, Mentored)

Details

(Whiteboard: [lang=js])

Attachments

(1 file)

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
can i take this one?
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 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 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+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/de31a15ecd05
Added Eslint file for the eslint-plugin-mozilla configs r=standard8
https://hg.mozilla.org/mozilla-central/rev/de31a15ecd05
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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.