Skip to content

tools: update eslint to latest#28173

Closed
BridgeAR wants to merge 4 commits intonodejs:masterfrom
BridgeAR:update-eslint-to-latest
Closed

tools: update eslint to latest#28173
BridgeAR wants to merge 4 commits intonodejs:masterfrom
BridgeAR:update-eslint-to-latest

Conversation

@BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Jun 11, 2019

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 11, 2019

Sadly, an error occurred when I tried to trigger a build. :(

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jun 11, 2019
targos
targos approved these changes Jun 11, 2019
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

RSLGTM

trivikr
trivikr approved these changes Jun 11, 2019
View reviewed changes
Copy link
Member

@trivikr trivikr left a comment

Choose a reason for hiding this comment

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

RSLGTM

@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 11, 2019

Lite CI https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3674/

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 11, 2019
@Trott
Copy link
Member

Trott commented Jun 12, 2019 •
edited
Loading

Nit 1: Please specify what version you are upgrading to in the commit message. (Preferably what version we're upgrading from as well.)

Nit 2: If I'm not mistaken, unneeded disable comments will result in errors with our configuration. If I'm right about that, then make lint-js will fail when the first commit lands but the second does not. If that's the case (and again, I believe it is), please squash the two commits into a single commit so that there isn't a commit where linting fails.

@BridgeAR BridgeAR force-pushed the update-eslint-to-latest branch from 1739468 to 70e595c Compare June 12, 2019 09:18
@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 12, 2019

@Trott thanks, I forgot that it would complain about the comments if they are not required. I updated the PR and incorporated the suggestions.

Lite-CI https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3675/

@BridgeAR
tools: update eslint
0f116e5 
This updates eslint from v6.0.0-alpha.2 to v6.0.0-rc.0.

This also removes eslint-disable comments about `bigint` typeof
checks. Those would otherwise have caused linting errors now that
`bigint` is accepted as valid entry.
@BridgeAR BridgeAR force-pushed the update-eslint-to-latest branch from 70e595c to 0f116e5 Compare June 17, 2019 10:17
@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 17, 2019

Rebased due to conflicts.

Lite-CI https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3686/

@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 20, 2019

New CI https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3694/

@ZYSzys
Copy link
Member

ZYSzys commented Jun 22, 2019 •
edited
Loading

@BridgeAR eslint@6.0.0 was just released. How about updating to v6.0.0 directly ?

And update this line to eslint@latest ?

node/tools/update-eslint.sh

Line 16 in e4136c1

npm install --global-style --no-bin-links --production --no-package-lock eslint@next

Refs: https://github.com/eslint/eslint/releases/tag/v6.0.0

@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 22, 2019

Updated to the latest release.

ZYSzys
ZYSzys approved these changes Jun 23, 2019
View reviewed changes
Copy link
Member

@ZYSzys ZYSzys left a comment

Choose a reason for hiding this comment

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

Still RSLGTM.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 25, 2019

ESLint 6.0.1 was released. Please skip 6.0.0 before landing this.

@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 26, 2019

Lite CI https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3721/

BridgeAR added a commit to BridgeAR/node that referenced this pull request Jun 27, 2019
@BridgeAR
tools: update eslint
ed8fc7e 
This updates eslint from v6.0.0-alpha.2 to v6.0.1

This also removes eslint-disable comments about `bigint` typeof
checks. Those would otherwise have caused linting errors now that
`bigint` is accepted as valid entry.

PR-URL: nodejs#28173
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
@BridgeAR
Copy link
Member Author

BridgeAR commented Jun 27, 2019

Landed in ed8fc7e

@BridgeAR BridgeAR closed this Jun 27, 2019
targos pushed a commit that referenced this pull request Jul 2, 2019
@BridgeAR @targos
tools: update eslint
023c75e 
This updates eslint from v6.0.0-alpha.2 to v6.0.1

This also removes eslint-disable comments about `bigint` typeof
checks. Those would otherwise have caused linting errors now that
`bigint` is accepted as valid entry.

PR-URL: #28173
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
targos pushed a commit that referenced this pull request Jul 2, 2019
@BridgeAR @targos
tools: update eslint
b744bd9 
This updates eslint from v6.0.0-alpha.2 to v6.0.1

This also removes eslint-disable comments about `bigint` typeof
checks. Those would otherwise have caused linting errors now that
`bigint` is accepted as valid entry.

PR-URL: #28173
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
@targos targos mentioned this pull request Jul 2, 2019
Merged
@beevelop beevelop mentioned this pull request Jul 3, 2019
Closed
This was referenced Jul 3, 2019
Closed
Closed
@BridgeAR BridgeAR deleted the update-eslint-to-latest branch January 20, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes

@targos targos targos approved these changes

@trivikr trivikr trivikr approved these changes

@ZYSzys ZYSzys ZYSzys approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@BridgeAR @nodejs-github-bot @Trott @ZYSzys @cjihrig @targos @trivikr