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.

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

build,win: include addons-napi in linter #16181

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 13, 2017

Currently test/addons-napi files are not being included in the lint
processing. This commit adds them.

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

build, windows

Currently test/addons-napi files are not being included in the lint
processing. This commit adds them.
@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 Oct 13, 2017
@danbev
Copy link
Contributor Author

danbev commented Oct 13, 2017

vcbuild.bat Outdated
@@ -502,6 +502,9 @@ if %errorlevel% equ 0 goto exit
echo %1 | findstr /r /c:"test\\addons\\[0-9].*_.*\.cc"
if %errorlevel% equ 0 goto exit

echo %1 | findstr /c:"test\\addons-napi\common.h"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't that be "test\\addons-napi\\common.h"? (Note the second double slash.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes I should, sorry missed that. I'll update. Thanks

@jasnell
Copy link
Member

jasnell commented Oct 15, 2017

@danbev
Copy link
Contributor Author

danbev commented Oct 16, 2017

test/windows-fanned unstable report looks unrelated

console output:

Checking ^not ok
c:\workspace\node-test-binary-windows\test.tap:
not ok 468 inspector/test-bindings # TODO : Fix flaky test
not ok 469 inspector/test-debug-end # TODO : Fix flaky test
Build step 'Jenkins Text Finder' changed build result to UNSTABLE
Notifying upstream projects of job completion
Finished: UNSTABLE

danbev added a commit to danbev/node that referenced this pull request Oct 16, 2017
Currently test/addons-napi files are not being included in the lint
processing. This commit adds them.

PR-URL: nodejs#16181
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented Oct 16, 2017

Landed in 7832e69

@danbev danbev closed this Oct 16, 2017
@danbev danbev deleted the win-napi-linter branch October 16, 2017 05:30
targos pushed a commit that referenced this pull request Oct 18, 2017
Currently test/addons-napi files are not being included in the lint
processing. This commit adds them.

PR-URL: #16181
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 18, 2017
Currently test/addons-napi files are not being included in the lint
processing. This commit adds them.

PR-URL: nodejs/node#16181
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants