Closed Bug 1171460 Opened 8 years ago Closed 8 years ago

ESLint: Reconfigure comma-dangle

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

Details

Attachments

(1 file)

The current setting of:

"comma-dangle": 1,

warns for trailing commas in objects, etc.

But, I find these are helpful in most cases, so that when you add a new item below, you don't have to worry about the previous item.

Anyway, to me I don't think this style point matters too much: it's fine to have them, and fine to not have them.
As discussed on IRC:

<pbro> jryans: I don't feel strongly at all about that rule, I don't think it's big deal if we are inconsistent on this, it doesn't hurt readability of the code, and can't be a runtime problem anyway. I disallowed it in the original version because we had 3 persons agree on that in the ehterpad
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Attachment #8615348 - Flags: review?(pbrosset)
Comment on attachment 8615348 [details] [diff] [review]
0001-Bug-1171460-ESLint-Allow-trailing-commas.-r-pbrosset.patch

Review of attachment 8615348 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/.eslintrc
@@ +31,5 @@
>      // start and end braces on the same line.
>      "brace-style": [2, "1tbs", {"allowSingleLine": false}],
>      // Require camel case names
>      "camelcase": 2,
> +    // Allow trailing commas.

Could you elaborate why we allow trailing commas in this comment? Mention that that they make it easy to add new items to lists, and that they don't hurt readability of the code, etc..
Attachment #8615348 - Flags: review?(pbrosset) → review+
I should add: I think these comments are very useful because they are basically our style-guide.
https://hg.mozilla.org/mozilla-central/rev/ec9aa7541e4a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.