test: skip doctool tests when js-yaml is missing - #7218
Conversation
Sorry, something went wrong.
|
LGTM, but we should eventually find a solution. What's js-yaml used for, by the way? Just the "added" comments? |
Sorry, something went wrong.
|
Hmmm, when will this be skipped? Only from the tarball, not on the CI? |
Sorry, something went wrong.
|
@Fishrock123 yes, only the tarball. There’s also #7290 that addresses the same problem by actually adding the dependencies, and I don’t have any too strong feelings, which is why I didn’t merge this yet. |
Sorry, something went wrong.
|
I personally prefer this approach. |
Sorry, something went wrong.
|
@thefourtheye Would you mind if I landed this? That doesn’t need to be the end of discussion, but it might be nice to have make test from tarballs working again. |
Sorry, something went wrong.
|
No problem. Working tarball is important. LGTM. |
Sorry, something went wrong.
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: nodejs#7201 Ref: nodejs#6495 PR-URL: nodejs#7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
|
rebased, CI again just to be sure: https://ci.nodejs.org/job/node-test-commit/3991/ |
Sorry, something went wrong.
|
@addaleax lts? If so can you backport? Not landing cleanly (thanks for being so responsive today!) |
Sorry, something went wrong.
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: nodejs#7201 Ref: nodejs#6495 PR-URL: nodejs#7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Skip the doctool tests when js-yaml, which is currently `require()`d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs. Fixes: #7201 Ref: #6495 PR-URL: #7218 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Checklist
Affected core subsystem(s)
test
Description of change
Skip the doctool tests when js-yaml, which is currently require()d from the eslint source tree, is missing. This can happen, for example, because eslint is not included in the release source tarballs.
Fixes: #7201
Ref (introduced in): #6495
CI: https://ci.nodejs.org/job/node-test-commit/3692/