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: update ESLint to current version #10561

Closed
wants to merge 4 commits into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 1, 2017

We have been stalled on ESLint 3.8.0 for some time. Current ESLint is
3.12.2. We have been unable to upgrade because of more aggressive
reporting on some rules, including indentation.

ESLint configuration options and bugfixes are now such that we can
reasonably upgrade.

ESLint `indent` rule now has options that duplicate functionality in our
custom `align-function-arguments` rule. Remove
`align-function-arguments` custom rule.

The `no-useless-regex-char-class-escape` custom lint rule was introduced
as a less aggressive alternative to some enhancements that were
introduced into ESLint. Those enhancements were blocking us from
updating ESLint. However, they have since been relaxed and the custom
rule is no longer needed. Remove it.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools benchmark test lib

@Trott Trott added benchmark Issues and PRs related to the benchmark subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Jan 1, 2017
@nodejs-github-bot nodejs-github-bot added module Issues and PRs related to the module subsystem. os Issues and PRs related to the os subsystem. repl Issues and PRs related to the REPL subsystem. util Issues and PRs related to the built-in util module. dont-land-on-v7.x labels Jan 1, 2017
@Trott
Copy link
Member Author

Trott commented Jan 1, 2017

Copy link
Contributor

@not-an-aardvark not-an-aardvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I noticed that this was installed from ESLint's latest master (eslint/eslint@c5c3b21), which is slightly ahead of the v3.12.2 release. I assume this is because eslint/eslint@f20b9e9, which we need in order to upgrade, hasn't made it into a release yet. Would it be worth waiting until Friday so that we can use a version that appears in a release?

(I'm not sure whether there is actually a compelling reason to wait, but I thought I'd raise the question.)

@gibfahn
Copy link
Member

gibfahn commented Jan 1, 2017

@not-an-aardvark If it's only going to be a few days, I'd say it's worth waiting, then we can just say eslint is v3.12.3 without having to talk about commits.

I'm assuming there's nothing that we urgently need in here...

@sam-github
Copy link
Contributor

options and bufixes are

"bugfixes"?

LGTM, but it would be better to use an officially released version rather than an install from master if possible.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber stamp lgtm

@jasnell
Copy link
Member

jasnell commented Jan 5, 2017

@Trott... did you see @sam-github's comment here #10561 (comment) with regards to using a released version vs. an image from master?

@Trott
Copy link
Member Author

Trott commented Jan 5, 2017

@Trott... did you see @sam-github's comment here #10561 (comment) with regards to using a released version vs. an image from master?

Waiting for an official release we can use. Should be coming out tomorrow, as ESLint does releases on Fridays. If there's no deal-breaker bugs for us, should be able to use that and merge this tomorrow. (Been tracking master on this branch specifically to catch bugs that will affect us sooner than the release.)

Formatting changes for upcoming linter update.
We have been stalled on ESLint 3.8.0 for some time. Current ESLint is
3.13.0. We have been unable to upgrade because of more aggressive
reporting on some rules, including indentation.

ESLint configuration options and bugfixes are now such that we can
reasonably upgrade.
ESLint `indent` rule now has options that duplicate functionality in our
custom `align-function-arguments` rule. Remove
`align-function-arguments` custom rule.
The `no-useless-regex-char-class-escape` custom lint rule was introduced
as a less aggressive alternative to some enhancements that were
introduced into ESLint. Those enhancements were blocking us from
updating ESLint. However, they have since been relaxed and the custom
rule is no longer needed. Remove it.
@Trott
Copy link
Member Author

Trott commented Jan 6, 2017

Trott added a commit to Trott/io.js that referenced this pull request Jan 6, 2017
Formatting changes for upcoming linter update.

PR-URL: nodejs#10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@MylesBorins
Copy link
Contributor

@Trott do you want to update this on LTS?

Trott added a commit to Trott/io.js that referenced this pull request Mar 9, 2017
Formatting changes for upcoming linter update.

PR-URL: nodejs#10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Mar 9, 2017
We have been stalled on ESLint 3.8.0 for some time. Current ESLint is
3.13.0. We have been unable to upgrade because of more aggressive
reporting on some rules, including indentation.

ESLint configuration options and bugfixes are now such that we can
reasonably upgrade.

PR-URL: nodejs#10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Mar 9, 2017
ESLint `indent` rule now has options that duplicate functionality in our
custom `align-function-arguments` rule. Remove
`align-function-arguments` custom rule.

PR-URL: nodejs#10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@Trott
Copy link
Member Author

Trott commented Mar 9, 2017

@MylesBorins Backport to v6.x is at #11757. Meanwhile, backport to v4.x probably isn't worth it at this point.

MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Formatting changes for upcoming linter update.

PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
We have been stalled on ESLint 3.8.0 for some time. Current ESLint is
3.13.0. We have been unable to upgrade because of more aggressive
reporting on some rules, including indentation.

ESLint configuration options and bugfixes are now such that we can
reasonably upgrade.

PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
ESLint `indent` rule now has options that duplicate functionality in our
custom `align-function-arguments` rule. Remove
`align-function-arguments` custom rule.

PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Formatting changes for upcoming linter update.

PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
We have been stalled on ESLint 3.8.0 for some time. Current ESLint is
3.13.0. We have been unable to upgrade because of more aggressive
reporting on some rules, including indentation.

ESLint configuration options and bugfixes are now such that we can
reasonably upgrade.

PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
ESLint `indent` rule now has options that duplicate functionality in our
custom `align-function-arguments` rule. Remove
`align-function-arguments` custom rule.

PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Trott added a commit to Trott/io.js that referenced this pull request Jul 19, 2017
The `no-useless-regex-char-class-escape` custom lint rule was introduced
as a less aggressive alternative to some enhancements that were
introduced into ESLint. Those enhancements were blocking us from
updating ESLint. However, they have since been relaxed and the custom
rule is no longer needed. Remove it.

PR-URL: nodejs#10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 21, 2017
The `no-useless-regex-char-class-escape` custom lint rule was introduced
as a less aggressive alternative to some enhancements that were
introduced into ESLint. Those enhancements were blocking us from
updating ESLint. However, they have since been relaxed and the custom
rule is no longer needed. Remove it.

Backport-PR-URL: #14360
PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 21, 2017
The `no-useless-regex-char-class-escape` custom lint rule was introduced
as a less aggressive alternative to some enhancements that were
introduced into ESLint. Those enhancements were blocking us from
updating ESLint. However, they have since been relaxed and the custom
rule is no longer needed. Remove it.

Backport-PR-URL: #14360
PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 21, 2017
MylesBorins pushed a commit that referenced this pull request Jul 31, 2017
The `no-useless-regex-char-class-escape` custom lint rule was introduced
as a less aggressive alternative to some enhancements that were
introduced into ESLint. Those enhancements were blocking us from
updating ESLint. However, they have since been relaxed and the custom
rule is no longer needed. Remove it.

Backport-PR-URL: #14360
PR-URL: #10561
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@Trott Trott deleted the eslint-3.12.2 branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. module Issues and PRs related to the module subsystem. os Issues and PRs related to the os subsystem. repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants