digitalbazaar / eslint-config-digitalbazaar

BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Add rules concerning dangling commas #49

Closed aljones15 closed 3 years ago

aljones15 commented 3 years ago

https://eslint.org/docs/rules/comma-dangle

When I joined DB I was told not to use dangling commas like this:

{
  foo: true,
  bar: true,
}

however it appears this might not be the consensus on this issue.

Please discuss, and thumbs up if you are ok with this new rule:

/*eslint comma-dangle: ["error", "always"]*/

var foo = {
    bar: "baz",
    qux: "quux",
};
// no error
var arr = [1,2,];

foo({
  bar: "baz",
  qux: "quux",
});
// no error

var foo = {
    bar: "baz",
    qux: "quux"
};
// error

var arr = [1,2];

foo({
  bar: "baz",
  qux: "quux"
});
// error
mattcollier commented 3 years ago

There is an existing rule: https://github.com/digitalbazaar/eslint-config-digitalbazaar/blob/master/index.js#L14

mattcollier commented 3 years ago

And an old issue: https://github.com/digitalbazaar/eslint-config-digitalbazaar/issues/17

dlongley commented 3 years ago

-1. I'm not ok with making it an error to have no dangling comma.

aljones15 commented 3 years ago

I'm going to go ahead and close this provided we are at consensus that we are ok with a dangling comma on multi-line, but it is also ok to not have a dangling comma if the dev chooses.

dlongley commented 3 years ago

Yup, I think that's the consensus we have (i.e., that we don't have consensus to use/not use dangling commas on multilines, so we allow devs to choose).

aljones15 commented 3 years ago

@mattcollier do you agree with @dlongley here? if so I'll close this.

aljones15 commented 3 years ago

48 hours is enough time. I'm closing this and assuming that as a company we allow developers to have dangling commas or not at their discretion. I also agree with that rule.