Skip to content

tools: fix order of ESLint rules#13363

Closed
targos wants to merge 1 commit intonodejs:masterfrom
targos:eslint-rule-order
Closed

tools: fix order of ESLint rules#13363
targos wants to merge 1 commit intonodejs:masterfrom
targos:eslint-rule-order

Conversation

@targos
Copy link
Copy Markdown
Member

@targos targos commented Jun 1, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jun 1, 2017
charmander
charmander reviewed Jun 1, 2017
View reviewed changes
Comment thread .eslintrc.yaml Outdated
Copy link
Copy Markdown
Contributor

@charmander charmander Jun 1, 2017

Choose a reason for hiding this comment

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

Should the comma here also be removed?

Copy link
Copy Markdown
Member Author

@targos targos Jun 1, 2017

Choose a reason for hiding this comment

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

You are right. Done.

@targos targos force-pushed the eslint-rule-order branch from 59651b4 to 818d46b Compare June 1, 2017 09:04
mhdawson
mhdawson approved these changes Jun 1, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

Trott
Trott approved these changes Jun 1, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM if linter is green

@watilde
Copy link
Copy Markdown
Member

watilde commented Jun 2, 2017

lint ci: https://ci.nodejs.org/job/node-test-linter/9544/

targos added a commit to targos/node that referenced this pull request Jun 6, 2017
@targos
tools: fix order of ESLint rules
f1199c2 
PR-URL: nodejs#13363
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@targos
Copy link
Copy Markdown
Member Author

targos commented Jun 6, 2017

Landed in f1199c2

@targos targos closed this Jun 6, 2017
@targos targos deleted the eslint-rule-order branch June 6, 2017 08:20
jasnell pushed a commit that referenced this pull request Jun 7, 2017
@targos @jasnell
tools: fix order of ESLint rules
01278bd 
PR-URL: #13363
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@watilde watilde watilde approved these changes

@cjihrig cjihrig cjihrig approved these changes

+4 more reviewers

@charmander charmander charmander left review comments

@silverwind silverwind silverwind approved these changes

@mhdawson mhdawson mhdawson approved these changes

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

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.
⚡