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.

Sign up for GitHub

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

Jump to bottom

tools: enable linting for chained properties #7999

Closed
wants to merge 2 commits into from
Closed

tools: enable linting for chained properties #7999

wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Aug 6, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

Description of change

Enforces linting for indentation of multiline chained properties. Requires updating to ESLint 3.2.2.

Refs: #7920

@Trott Trott added the tools Issues and PRs related to the tools directory. label Aug 6, 2016
@Trott
Copy link
Member Author

Trott commented Aug 6, 2016

(eslint was installed with --production and dmn -f clean was run on the results.)

@Trott
Copy link
Member Author

Trott commented Aug 6, 2016

/cc @silverwind

@Trott
Copy link
Member Author

Trott commented Aug 7, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/3557/

@Trott
Copy link
Member Author

Trott commented Aug 8, 2016

Raspberry Pi build-type failures in Ci. Certainly unrelated. But let's re-run anyway: https://ci.nodejs.org/job/node-test-pull-request/3560/

@cjihrig
Copy link
Contributor

cjihrig commented Aug 8, 2016

Rubber stamp LGTM on the ESLint update. Actual LGTM on the other commit.

@silverwind
Copy link
Contributor

LGTM

2 similar comments
@jasnell
Copy link
Member

jasnell commented Aug 8, 2016

LGTM

@jbergstroem
Copy link
Member

LGTM

@jasnell
Copy link
Member

jasnell commented Aug 9, 2016

More flaky failures in that last CI run but still unrelated. CI has been particularly flaky here this past week :-( /cc @nodejs/build

Will go ahead and land this but definitely need to keep an eye on things.

@jasnell
Copy link
Member

jasnell commented Aug 9, 2016

actually... hmmm.. @Trott, after applying this patch locally I'm getting:

$ make lint
Linting is not available through the source tarball.
Use the git repo instead: git clone https://github.com/nodejs/node.git

When I pull this patch back out it works.

@Trott
Copy link
Member Author

Trott commented Aug 9, 2016

Oh, interesting. The Makefile looks for the bin/eslint.js even though that file is not actually used in linting. I'll fix it.

@Trott
Copy link
Member Author

Trott commented Aug 9, 2016

OK, fixed make lint.

CI: https://ci.nodejs.org/job/node-test-pull-request/3603/

Trott added a commit to Trott/io.js that referenced this pull request Aug 10, 2016
@Trott
tools: update to ESLint 3.2.2
e313c02 
PR-URL: nodejs#7999
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Trott added a commit to Trott/io.js that referenced this pull request Aug 10, 2016
@Trott
tools: enable linting for chained properties
8726a1c 
Refs: nodejs#7920
PR-URL: nodejs#7999
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Trott
Copy link
Member Author

Trott commented Aug 10, 2016

Landed in e313c02 and 8726a1c

@Trott Trott closed this Aug 10, 2016
@cjihrig cjihrig mentioned this pull request Aug 10, 2016
Closed
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
@Trott @cjihrig
tools: update to ESLint 3.2.2
60ff991 
PR-URL: #7999
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
@Trott @cjihrig
tools: enable linting for chained properties
97934f9 
Refs: #7920
PR-URL: #7999
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
Merged
@Trott Trott deleted the eslint3.2.2 branch October 10, 2021 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Trott @cjihrig @silverwind @jasnell @jbergstroem @MylesBorins