Skip to content

Conversation

@joyeecheung
Copy link
Member

Prepping for nodejs/build#929

  • Implements the make test-doc target that build, verify and lint docs
  • Lint the C++ snippets in addon docs
  • When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

test, tools

@nodejs-github-botnodejs-github-bot added the build Issues and PRs related to build files or the CI. label Oct 22, 2017
@joyeecheung
Copy link
MemberAuthor

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Oct 22, 2017

BTW, why we abstain from making docs and running recently added doc tests on Windows? Is it from vcbuild.bat complication? It would be handy to have an option to make test doc builds on Windows locally.

cc @nodejs/platform-windows

@gibfahn
Copy link
Member

BTW, why we abstain from making docs and running recently added doc tests on Windows? Is it from vcbuild.bat complication? It would be handy to have an option to make test doc builds on Windows locally.

I think it's just waiting for someone to implement.

@gibfahn
Copy link
Member

@joyeecheung so what's the difference between make test-doc and make lint-md (#12756)?

Is test-doc going to call lint-md?

@joyeecheung
Copy link
MemberAuthor

@gibfahn That's the plan

Makefile Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why just here? shouldn't this be handled in a global manner? Or just err with the message "to run with a precompiled node binary run make NODE=<path_to_node> <target>"

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Good idea

@joyeecheung
Copy link
MemberAuthor

joyeecheung commented Oct 28, 2017

Figured out why the linter failed (ci doesn't build the addon docs before linting them).

@refack I gave the global $(NODE) a try and turns out it's trickier than I thought. Many rules use this pattern but some of them depend on the actual build rule indirectly, so the status of -x ./node might change. I'll separate the refactor into another PR.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

is this a lint issue?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Yes, needs two spaces here

@refack
Copy link
Contributor

Change looks good, but I'm assuming it's not well covered by CI?
I guess it would be nice if someone manually checked the changed targets (me Windows, no good make tester):

  • test
  • (test/addons/.docbuildstamp)
  • test-ci (CI)
  • test-doc
  • lint-js
  • lint-js-ci (CI)
  • lint-addon-docs
  • lint-ci (CI)

- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc.
@joyeecheung
Copy link
MemberAuthor

Rebased & squashed. New CI: https://ci.nodejs.org/job/node-test-pull-request/11063/

@joyeecheung
Copy link
MemberAuthor

Going to land this later today.

joyeecheung added a commit that referenced this pull request Oct 30, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: #16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@joyeecheung
Copy link
MemberAuthor

Landed in 390eda1, thanks!

gibfahn pushed a commit that referenced this pull request Oct 30, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: #16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: #16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
gibfahn pushed a commit that referenced this pull request Oct 31, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: #16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@gibfahngibfahn mentioned this pull request Oct 31, 2017
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: nodejs/node#16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: nodejs/node#16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: nodejs#16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@cjihrigcjihrig mentioned this pull request Nov 6, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
- Implements the make test-doc target that build, verify and lint docs - Lint the C++ snippets in addon docs - When generating addons and running the JS linter, use the global node executable if it is not built. Therefore one does not have to build node in order to run make test-doc. PR-URL: nodejs/node#16377 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@MylesBorinsMylesBorins added the baking-for-lts PRs that need to wait before landing in a LTS release. label Dec 20, 2017
@MylesBorinsMylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@joyeecheung@vsemozhetbyt@gibfahn@refack@jasnell@MylesBorins@nodejs-github-bot