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,test: enforce assert.ifError with lint rule #10671

Closed
wants to merge 1 commit into from
Closed

tools,test: enforce assert.ifError with lint rule #10671

wants to merge 1 commit into from

Conversation

not-an-aardvark
Copy link
Contributor

This adds an ESLint rule to enforce the use of assert.ifError(err)
instead of if (err) throw err; in tests.

Refs: #10543

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

tools test

cc @Trott

@not-an-aardvark
tools,test: enforce assert.ifError with lint rule
ef5b150 
This adds an ESLint rule to enforce the use of `assert.ifError(err)`
instead of `if (err) throw err;` in tests.

Refs: nodejs#10543
@nodejs-github-bot nodejs-github-bot added tools Issues and PRs related to the tools directory. dont-land-on-v7.x labels Jan 7, 2017
Trott
Trott approved these changes Jan 7, 2017
View reviewed changes
Copy link
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 CI is ✅

@not-an-aardvark
Copy link
Contributor Author

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

gibfahn
gibfahn approved these changes Jan 7, 2017
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Awesome, thanks for doing this @not-an-aardvark

CI is green

jasnell pushed a commit that referenced this pull request Jan 9, 2017
@not-an-aardvark @jasnell
tools,test: enforce assert.ifError with lint rule
5d31448 
This adds an ESLint rule to enforce the use of `assert.ifError(err)`
instead of `if (err) throw err;` in tests.

PR-URL: #10671
Ref: #10543
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 9, 2017

Landed in 5d31448

@jasnell jasnell closed this Jan 9, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
@not-an-aardvark @italoacasas
tools,test: enforce assert.ifError with lint rule
949c24e 
This adds an ESLint rule to enforce the use of `assert.ifError(err)`
instead of `if (err) throw err;` in tests.

PR-URL: nodejs#10671
Ref: nodejs#10543
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Closed
targos pushed a commit that referenced this pull request Jan 28, 2017
@not-an-aardvark @targos
tools,test: enforce assert.ifError with lint rule
af98924 
This adds an ESLint rule to enforce the use of `assert.ifError(err)`
instead of `if (err) throw err;` in tests.

PR-URL: #10671
Ref: #10543
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
Merged
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@not-an-aardvark @italoacasas
tools,test: enforce assert.ifError with lint rule
3adda4b 
This adds an ESLint rule to enforce the use of `assert.ifError(err)`
instead of `if (err) throw err;` in tests.

PR-URL: nodejs#10671
Ref: nodejs#10543
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@not-an-aardvark @italoacasas
tools,test: enforce assert.ifError with lint rule
fac607c 
This adds an ESLint rule to enforce the use of `assert.ifError(err)`
instead of `if (err) throw err;` in tests.

PR-URL: nodejs#10671
Ref: nodejs#10543
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 8, 2017

This will need backport PRs in order to land on v6 or v4

@not-an-aardvark not-an-aardvark deleted the assert-iferror branch April 4, 2017 02:17
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

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@gibfahn gibfahn gibfahn approved these changes

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.
8 participants
@not-an-aardvark @jasnell @Trott @targos @cjihrig @gibfahn @mscdex @nodejs-github-bot