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

build: fail linter if linting not available #13658

Closed
wants to merge 1 commit into from
Closed

build: fail linter if linting not available #13658

wants to merge 1 commit into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Jun 13, 2017

Refs: #13645 (comment)

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

build, lint

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Jun 13, 2017
@gibfahn gibfahn mentioned this pull request Jun 13, 2017
Closed
2 tasks
joaocgreis
joaocgreis approved these changes Jun 13, 2017
View reviewed changes
vcbuild.bat Outdated
@@ -485,6 +485,7 @@ goto exit
:no-lint
echo Linting is not available through the source tarball.
echo Use the git repo instead: $ git clone https://github.com/nodejs/node.git
exit /b 1
goto exit
Copy link
Member

Choose a reason for hiding this comment

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

This goto can be removed.

refack
refack approved these changes Jun 13, 2017
View reviewed changes
vcbuild.bat Outdated
@@ -485,6 +485,7 @@ goto exit
:no-lint
echo Linting is not available through the source tarball.
echo Use the git repo instead: $ git clone https://github.com/nodejs/node.git
exit /b 1
goto exit
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: remove redundant goto

Copy link
Contributor

Choose a reason for hiding this comment

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

What's up with GitHub, why didn't I see this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done twice 😄

jasnell pushed a commit that referenced this pull request Jun 16, 2017
@gibfahn @jasnell
build: fail linter if linting not available
b7473c2 
PR-URL: #13658
Ref: #13645 (comment)
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jun 16, 2017

Landed in b7473c2

@jasnell jasnell closed this Jun 16, 2017
@gibfahn gibfahn deleted the fail-lint branch June 16, 2017 21:59
addaleax pushed a commit that referenced this pull request Jun 17, 2017
@gibfahn @addaleax
build: fail linter if linting not available
aad282f 
PR-URL: #13658
Ref: #13645 (comment)
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@addaleax addaleax mentioned this pull request Jun 17, 2017
Merged
addaleax pushed a commit that referenced this pull request Jun 21, 2017
@gibfahn @addaleax
build: fail linter if linting not available
18f073f 
PR-URL: #13658
Ref: #13645 (comment)
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@addaleax addaleax mentioned this pull request Jun 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
@gibfahn @MylesBorins
build: fail linter if linting not available
f571868 
PR-URL: #13658
Ref: #13645 (comment)
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Merged
@jellelicht jellelicht mentioned this pull request Jul 27, 2017
Closed
gibfahn added a commit to gibfahn/node that referenced this pull request Sep 16, 2017
@gibfahn
build: don't fail make test on source tarballs
341bcfe 
Tries to achieve the same effect as
nodejs#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

Fixes: nodejs#14513
@gibfahn gibfahn mentioned this pull request Sep 16, 2017
Closed
2 tasks
BridgeAR pushed a commit that referenced this pull request Sep 21, 2017
@gibfahn @BridgeAR
build: don't fail make test on source tarballs
66e45b8 
Tries to achieve the same effect as
#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

PR-URL: #15441
Fixes: #14513
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
lukaszewczak pushed a commit to lukaszewczak/node that referenced this pull request Sep 23, 2017
@gibfahn @lukaszewczak
build: don't fail make test on source tarballs
5d611c3 
Tries to achieve the same effect as
nodejs#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

PR-URL: nodejs#15441
Fixes: nodejs#14513
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 23, 2017
@gibfahn @addaleax
build: don't fail make test on source tarballs
cae02ce 
Tries to achieve the same effect as
nodejs/node#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

PR-URL: nodejs/node#15441
Fixes: nodejs/node#14513
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this pull request Sep 25, 2017
@gibfahn @jasnell
build: don't fail make test on source tarballs
9685b9f 
Tries to achieve the same effect as
#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

PR-URL: #15441
Fixes: #14513
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 17, 2017
@gibfahn @MylesBorins
build: don't fail make test on source tarballs
8f1a586 
Tries to achieve the same effect as
#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

PR-URL: #15441
Fixes: #14513
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@gibfahn @MylesBorins
build: don't fail make test on source tarballs
6c9a9ff 
Tries to achieve the same effect as
#13658 without breaking source
tarballs. Presumably if `tools/eslint` wasn't there at all, people
would notice in the PR review!

PR-URL: #15441
Fixes: #14513
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasontedor jasontedor mentioned this pull request Nov 15, 2017
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

@joaocgreis joaocgreis joaocgreis approved these changes

@silverwind silverwind silverwind approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@gibfahn @jasnell @refack @silverwind @joaocgreis @cjihrig @tniessen @MylesBorins @nodejs-github-bot