Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: enable additional lint rules #5357

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Feb 22, 2016

Enable additional likely-uncontroversial lint rules:

  • comma-dangle set to prohibit dangling commas on objects and arrays
    that are defined on a single line. Multi-line definitions can use or
    omit a trailing comma.
  • no-unused-labels Prohibits defining a label that is not used.
  • no-path-concat Prohibits string-concatenation using __dirname and
    __filename. Use path.join(), path.resolve(), or template strings
    instead.
  • no-new-symbol disallow use of new operator with Symbol object.
    Violating this rule would result in a TypeError at runtime.

Enable additional likely-uncontroversial lint rules:

* `comma-dangle` set to prohibit dangling commas on objects and arrays
that are defined on a single line. Multi-line definitions can use or
omit a trailing comma.

* `no-unused-labels` Prohibits defining a label that is not used.

* `no-path-concat` Prohibits string-concatenation using i`__dirname` and
`__filename`. Use `path.join()`, `path.resolve()`, or template strings
instead.

* `no-new-symbol` disallow use of `new` operator with `Symbol` object.
Violating this rule would result in a `TypeError` at runtime.`
@Trott Trott added tools Issues and PRs related to the tools directory. lts-watch-v4.x labels Feb 22, 2016
@targos
Copy link
Member

targos commented Feb 22, 2016

@cjihrig
Copy link
Contributor

cjihrig commented Feb 22, 2016

@rvagg
Copy link
Member

rvagg commented Feb 23, 2016

I had to look up comma-dangle to see what the implications are, for anyone else interested: http://eslint.org/docs/rules/comma-dangle

The following patterns are considered problems when configured "only-multiline":

/*eslint comma-dangle: [1, "only-multiline"]*/

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

var arr = [1,2,];

var arr = [1,
    2,];

But things like this are still OK:

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

var arr = [1,
    2];

I can't find the previous discussion about dangling commas, if you know where it was would you mind @-mentioning anyone that objected to disallowing dangling commas (my recollection of that discussion) to give them a chance to weigh in here @Trott?

LGTM

@Trott
Copy link
Member Author

Trott commented Feb 23, 2016

@rvagg Sure! Although the objection was to the default comma-dangle behavior which flagged about 80 or so lines of our current code base. This far more lenient version flags zero lines of our current code base.

Should be non-controversial, I think, but just in case, as requested by @rvagg, @-mentioning previous objectors: @Fishrock123 @bnoordhuis

Previous discussion is here: #5091

@Fishrock123
Copy link
Contributor

I'm ok with only-multiline.

Trott added a commit to Trott/io.js that referenced this pull request Feb 24, 2016
Enable additional likely-uncontroversial lint rules:

* `comma-dangle` set to prohibit dangling commas on objects and arrays
that are defined on a single line. Multi-line definitions can use or
omit a trailing comma.

* `no-unused-labels` Prohibits defining a label that is not used.

* `no-path-concat` Prohibits string-concatenation using i`__dirname` and
`__filename`. Use `path.join()`, `path.resolve()`, or template strings
instead.

* `no-new-symbol` disallow use of `new` operator with `Symbol` object.
Violating this rule would result in a `TypeError` at runtime.`

PR-URL: nodejs#5357
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@Trott
Copy link
Member Author

Trott commented Feb 24, 2016

Landed in 9534f6d

@Trott Trott closed this Feb 24, 2016
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Enable additional likely-uncontroversial lint rules:

* `comma-dangle` set to prohibit dangling commas on objects and arrays
that are defined on a single line. Multi-line definitions can use or
omit a trailing comma.

* `no-unused-labels` Prohibits defining a label that is not used.

* `no-path-concat` Prohibits string-concatenation using i`__dirname` and
`__filename`. Use `path.join()`, `path.resolve()`, or template strings
instead.

* `no-new-symbol` disallow use of `new` operator with `Symbol` object.
Violating this rule would result in a `TypeError` at runtime.`

PR-URL: #5357
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Enable additional likely-uncontroversial lint rules:

* `comma-dangle` set to prohibit dangling commas on objects and arrays
that are defined on a single line. Multi-line definitions can use or
omit a trailing comma.

* `no-unused-labels` Prohibits defining a label that is not used.

* `no-path-concat` Prohibits string-concatenation using i`__dirname` and
`__filename`. Use `path.join()`, `path.resolve()`, or template strings
instead.

* `no-new-symbol` disallow use of `new` operator with `Symbol` object.
Violating this rule would result in a `TypeError` at runtime.`

PR-URL: #5357
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks
MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
Enable additional likely-uncontroversial lint rules:

* `comma-dangle` set to prohibit dangling commas on objects and arrays
that are defined on a single line. Multi-line definitions can use or
omit a trailing comma.

* `no-unused-labels` Prohibits defining a label that is not used.

* `no-path-concat` Prohibits string-concatenation using i`__dirname` and
`__filename`. Use `path.join()`, `path.resolve()`, or template strings
instead.

* `no-new-symbol` disallow use of `new` operator with `Symbol` object.
Violating this rule would result in a `TypeError` at runtime.`

PR-URL: #5357
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
Enable additional likely-uncontroversial lint rules:

* `comma-dangle` set to prohibit dangling commas on objects and arrays
that are defined on a single line. Multi-line definitions can use or
omit a trailing comma.

* `no-unused-labels` Prohibits defining a label that is not used.

* `no-path-concat` Prohibits string-concatenation using i`__dirname` and
`__filename`. Use `path.join()`, `path.resolve()`, or template strings
instead.

* `no-new-symbol` disallow use of `new` operator with `Symbol` object.
Violating this rule would result in a `TypeError` at runtime.`

PR-URL: #5357
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@Trott Trott deleted the moar-rules branch January 13, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants