1486741 - Enable ESLint rule comma-dangle for all of mozilla-central

Status: RESOLVED FIXED

(Core :: General, enhancement, P1)

Description

[Mark Banner (:standard8)]

As discussed on firefox-dev and dev.platform, we should require dangling commas for multi-line object/arrays in Javascript code for mozilla-central.

Why:

- Multiple components in toolkit/ and browser/ already have comma-dangle enabled.
- Having asked around in a few locations, I'm seeing support from the Firefox developer team and others in favour of enabling it.
- It helps make blame cleaner.
- It makes editing easier, and helps for a consistent style.
- Having automation available to cover the requirement reduces the need for review nits.

This bug is for the second half of the files, those not already enabled by bug 1486739.

I am hoping to land this bug on Friday 19 October (GMT timezone), ahead of the 64 merges.

Comment 1

[Jeff Walden [:Waldo]]

This auto-rewriting won't touch anything in js/src/tests or js/src/jit-test, will it?  Absence of a trailing comma can't be presumed to be meaningless there, and much of the former is imported anyway.

Comment 2

[Mark Banner (:standard8)]

(In reply to Jeff Walden [:Waldo] from comment #1)
> This auto-rewriting won't touch anything in js/src/tests or js/src/jit-test,
> will it?  Absence of a trailing comma can't be presumed to be meaningless
> there, and much of the former is imported anyway.

They are ignored by default for ESLint, so they won't be touched: https://searchfox.org/mozilla-central/rev/55da592d85c2baf8d8818010c41d9738c97013d2/.eslintignore#284,292,294

Comment 3

[Andreas Tolfsen ❲:ato❳]

I hope this change will also be applied to /testing?  Specifically
for testing/marionette we want to adopt as much of the repository-wide
lint rules as possible.

Comment 4

[Mark Banner (:standard8)]

(In reply to Andreas Tolfsen ﹝:ato﹞ from comment #3)
> I hope this change will also be applied to /testing?  Specifically
> for testing/marionette we want to adopt as much of the repository-wide
> lint rules as possible.

It will be applied for everywhere that ESLint is enabled (i.e. anywhere not ignored by .eslintignore). Note: comma-dangle is already enabled for testing/marionette, so all that will happen there is the sub-directory specific configuration will be removed in preference to the main one (tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js).

Comment 5

[Mark Banner (:standard8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a9bcbb2615df11c08371ea5722ab21e64db0d7a

Comment 6

[Mark Banner (:standard8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba38826371ea7a79a72322eb39a92534f5295968

Comment 7

[Mark Banner (:standard8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=50381fa64622383d3e972f5940b8160c1a6500ee

Comment 8

[Mark Banner (:standard8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=43f37dc9e3527d76232e796b30f08db85bf4b9dd

Comment 9

[Mark Banner (:standard8)]

Attachment: Bug 1486741 - Enable ESLint rule comma-dangle for all of mozilla-central (setup ESLint).

Comment 10

[Mark Banner (:standard8)]

Attachment: Bug 1486741 - Enable ESLint rule comma-dangle for all of mozilla-central (automatic fixes).

Comment 11

[Mark Banner (:standard8)]

Attachment: Bug 1486741 - Enable ESLint rule comma-dangle for all of mozilla-central (manual fixes).

Depends on D8389

Comment 12

[Mark Banner (:standard8)]

Attachment: Bug 1486741 - Enable ESLint rule comma-dangle for all of mozilla-central (setup ESLint).

Depends on D8391

Comment 13

[Mark Banner (:standard8)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b602cb2eb9e8a6102beff52b1e016ff88a293a0d&selectedJob=204791782

Comment 14

[Mark Banner (:standard8)]

Note: Aiming for landing next Friday (19th Oct)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/edc4ae8f78e2
Enable ESLint rule comma-dangle for all of mozilla-central (automatic fixes). r=mossop
https://hg.mozilla.org/integration/autoland/rev/988056e6d054
Enable ESLint rule comma-dangle for all of mozilla-central (manual fixes). r=mossop
https://hg.mozilla.org/integration/autoland/rev/1ac84aa579d6
Enable ESLint rule comma-dangle for all of mozilla-central (setup ESLint). r=mossop

Comment 16

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/edc4ae8f78e2
https://hg.mozilla.org/mozilla-central/rev/988056e6d054
https://hg.mozilla.org/mozilla-central/rev/1ac84aa579d6

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/345543cd8045