1376347 - Enable the ESLint func-call-spacing, key-spacing, keyword-spacing, and no-trailing-spaces rules for accessible/

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Mark Banner (:standard8)]

We currently have the rules enabled for mozilla-central (in recommended.js), but we should enable them for the accessible/ directory as well to provide more consistency with our eslint configuration.

I'm happy to mentor this bug. There's background on our eslint setups here:

https://developer.mozilla.org/docs/ESLint

Also, details of the rule: http://eslint.org/docs/rules/dot-notation

Here's some approximate steps:

- In accessible/.eslintrc.js remove the lines for:

* func-call-spacing
* key-spacing
* keyword-spacing
* no-trailing spaces

- Run eslint for fixing:

./mach eslint --fix

This should fix most/all of the instances.

- Fix any remaining instances by hand
- Inspect the diff to make sure that the indentation of the lines surrounding the changes look OK still.
- Create a commit and push it to mozreview:

http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html

Comment 1

[Hemant Singh Patwal]

Hi, Can I take this one. :)

Comment 2

[Hemant Singh Patwal]

Attachment: Bug 1376347 - Enable the ESLint func-call-spacing, key-spacing, keyword-spacing, and no-trailing-spaces rules for accessible/

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

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8882560 [details]
Bug 1376347 - Enable the ESLint func-call-spacing, key-spacing, keyword-spacing, and no-trailing-spaces rules for accessible/

https://reviewboard.mozilla.org/r/153666/#review159128

`./mach eslint accessible/` is still showing failures in `accessible/tests/mochitest/attributes/test_obj.html`, so you'll need to fix those by hand.

::: accessible/jsat/OutputGenerator.jsm:688
(Diff revision 1)
>        return utterance;
>      },
>  
>      list: function list(aAccessible, aRoleStr, aState, aFlags) {
> -      return this._getListUtterance
> -        (aAccessible, aRoleStr, aFlags, aAccessible.childCount);
> +       return this._getListUtterance(aAccessible, aRoleStr, aFlags,
> +          aAccessible.childCount);

nit: still needs 2-space based indentation here.

::: accessible/jsat/OutputGenerator.jsm:694
(Diff revision 1)
>      },
>  
>      definitionlist:
>        function definitionlist(aAccessible, aRoleStr, aState, aFlags) {
> -        return this._getListUtterance
> -          (aAccessible, aRoleStr, aFlags, aAccessible.childCount / 2);
> +        return this._getListUtterance(aAccessible, aRoleStr, aFlags,
> +           aAccessible.childCount / 2);

nit: still needs 2-space based indentation here.

Comment 4

[Mark Banner (:standard8)]

Alexander, do you want to do the final review on this once it is updated, or are you happy for me to do it?

Comment 5

[Marco Zehe (:MarcoZ)]

Surkov is out until next week, so to get this unblocking, I'm happy to do a final reviewif you feel like giving me one. ;)

Comment 6

[Hemant Singh Patwal]

Comment on attachment 8882560 [details]
Bug 1376347 - Enable the ESLint func-call-spacing, key-spacing, keyword-spacing, and no-trailing-spaces rules for accessible/

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

Comment 7

[Mark Banner (:standard8)]

Marco, I've taken a quick scan and it looks reasonable, so over to you :-) Thanks.

Comment 8

[Marco Zehe (:MarcoZ)]

Comment on attachment 8882560 [details]
Bug 1376347 - Enable the ESLint func-call-spacing, key-spacing, keyword-spacing, and no-trailing-spaces rules for accessible/

https://reviewboard.mozilla.org/r/153666/#review159538

Thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cfe257e5d880
Enable the ESLint func-call-spacing, key-spacing, keyword-spacing, and no-trailing-spaces rules for accessible/ r=MarcoZ

Comment 10

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

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