Proposal: Require dangling commas for multiline objects/arrays on all Javascript mozilla-central code

Mark Banner mbanner at mozilla.com
Wed Aug 22 20:29:26 UTC 2018


I would like to propose that we 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
    <https://searchfox.org/mozilla-central/search?q=comma-dangle&case=false&regexp=false&path=>.
  * 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.

I realise not everyone necessarily loves the style, but having the 
cleaner blame, and consistent style requirements seems to outweigh the 
downsides.

How:

  * We would enable this via an ESLint rule, comma-dangle
    <https://eslint.org/docs/rules/comma-dangle>, with the
    "always-multiline" option set.
  * ESLint is able to automatically fix code that doesn't conform (e.g.
    bug 1476228 <https://bugzilla.mozilla.org/show_bug.cgi?id=1476228>
    was entirely automatically generated very quickly)

Since there are a large amount of instances (approx 12k) that would need 
to be fixed across the code base, I would propose that we roll it out on 
a per-directory basis. For example, we could do all of browser/ at the 
start of the 64 cycle just after the merges (alternately the end of the 
63 cycle during soft freeze), then do toolkit/ at the next merge time, 
and then pick up the rest at the end. This will limit the scope of 
possible bitrotting to smaller chunks, as well as making the patch sizes 
more manageable.

This will only cover directories that ESLint currently covers - though 
as we roll out ESLint to new directories, they'll be covered automatically.

Feedback & thoughts welcome.

Standard8

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/firefox-dev/attachments/20180822/02cfb17b/attachment.html>


More information about the firefox-dev mailing list