From 689b2f50e528d45c9d2e9b31da374b8d0b580f01 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 12 Oct 2020 14:27:59 +0200 Subject: [PATCH 1/3] Run vale linter only if vale is installed --- package.json | 3 +- scripts/check-docs-quality.js | 86 ++++++++++++++++++++--------------- yarn.lock | 5 ++ 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/package.json b/package.json index 371646074c..96e02e33f9 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "@changesets/cli": "^2.11.0", "@spotify/eslint-config-oss": "^1.0.1", "@spotify/prettier-config": "^8.0.0", + "command-exists": "^1.2.9", "concurrently": "^5.2.0", "fs-extra": "^9.0.0", "husky": "^4.2.3", @@ -65,7 +66,7 @@ "prettier --write" ], "*.md": [ - "vale" + "node ./scripts/check-docs-quality" ] }, "jest": { diff --git a/scripts/check-docs-quality.js b/scripts/check-docs-quality.js index 61968a2ac0..4f5f689105 100755 --- a/scripts/check-docs-quality.js +++ b/scripts/check-docs-quality.js @@ -16,6 +16,8 @@ */ const { execSync, spawnSync } = require('child_process'); const path = require('path'); +// eslint-disable-next-line import/no-extraneous-dependencies +const commandExists = require('command-exists'); const listFilesTrackedByGit = 'git ls-files'; @@ -23,46 +25,58 @@ const inheritStdIo = { stdio: 'inherit', }; +const LINT_SKIPPED_MESSAGE = + 'Skipping documentation quality check (vale not found). Install vale linter (https://docs.errata.ai/vale/install) to enable.\n'; const ERROR_MESSAGE = - 'Please install vale linter(https://docs.errata.ai/vale/install). Ignore this message if already installed.\n'; + 'Language linter (vale) generated errors. Please check the errors and review any markdown files that you changed.\n'; -// xargs is not supported by shx. -if (process.platform === 'win32') { - const validMDFilesCommand = `${listFilesTrackedByGit} | .\\node_modules\\.bin\\shx grep ".md"`; - try { - // get list of all md files except in directories of gitignore. - let filesToLint = execSync(validMDFilesCommand, { - stdio: ['ignore', 'pipe', 'inherit'], - }); +// Proceed with the script only if Vale linter is installed. Limit the friction and surprises caused by the script. +commandExists('vale') + .catch(() => { + console.log(LINT_SKIPPED_MESSAGE); + process.exit(0); + }) + .then(() => { + // xargs is not supported by shx. + if (process.platform === 'win32') { + const validMDFilesCommand = `${listFilesTrackedByGit} | .\\node_modules\\.bin\\shx grep ".md"`; + try { + // get list of all md files except in directories of gitignore. + let filesToLint = execSync(validMDFilesCommand, { + stdio: ['ignore', 'pipe', 'inherit'], + }); - // set all file(s) path as absolute path - filesToLint = filesToLint - .toString() - .split('\n') - .map(filepath => (filepath ? path.join(process.cwd(), filepath) : null)) - .filter(Boolean); + // set all file(s) path as absolute path + filesToLint = filesToLint + .toString() + .split('\n') + .map(filepath => + filepath ? path.join(process.cwd(), filepath) : null, + ) + .filter(Boolean); - const output = spawnSync('vale', filesToLint, inheritStdIo); + const output = spawnSync('vale', filesToLint, inheritStdIo); - // if the command does not succeed - if (output.status !== 0) { - // if it contains system level error. [in this case vale does not exist] - if (output.error) { - console.error(ERROR_MESSAGE); + // if the command does not succeed + if (output.status !== 0) { + // if it contains system level error. [in this case vale does not exist] + if (output.error) { + console.error(ERROR_MESSAGE); + } + process.exit(1); + } + } catch (e) { + console.error(e.message); + process.exit(1); + } + } else { + const validMDFilesCommand = `${listFilesTrackedByGit} | ./node_modules/.bin/shx grep ".md"`; + // use xargs + try { + execSync(`${validMDFilesCommand} | xargs vale`, inheritStdIo); + } catch (e) { + console.error(ERROR_MESSAGE); + process.exit(1); } - process.exit(1); } - } catch (e) { - console.error(e.message); - process.exit(1); - } -} else { - const validMDFilesCommand = `${listFilesTrackedByGit} | ./node_modules/.bin/shx grep ".md"`; - // use xargs - try { - execSync(`${validMDFilesCommand} | xargs vale`, inheritStdIo); - } catch (e) { - console.error(ERROR_MESSAGE); - process.exit(1); - } -} + }); diff --git a/yarn.lock b/yarn.lock index 7dbc7f964a..859c26c6c4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8537,6 +8537,11 @@ command-exists-promise@^2.0.2: resolved "https://registry.npmjs.org/command-exists-promise/-/command-exists-promise-2.0.2.tgz#7beecc4b218299f3c61fa69a4047aa0b36a64a99" integrity sha512-T6PB6vdFrwnHXg/I0kivM3DqaCGZLjjYSOe0a5WgFKcz1sOnmOeIjnhQPXVXX3QjVbLyTJ85lJkX6lUpukTzaA== +command-exists@^1.2.9: + version "1.2.9" + resolved "https://registry.npmjs.org/command-exists/-/command-exists-1.2.9.tgz#c50725af3808c8ab0260fd60b01fbfa25b954f69" + integrity sha512-LTQ/SGc+s0Xc0Fu5WaKnR0YiygZkm9eKFvyS+fRsU7/ZWFF8ykFM6Pc9aCVf1+xasOOZpO3BAVgVrKvsqKHV7w== + commander@^2.11.0, commander@^2.19.0, commander@^2.20.0, commander@^2.20.3, commander@~2.20.3: version "2.20.3" resolved "https://registry.npmjs.org/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33" From 3b17fa02f87eef8c3ea0a7a5ccf18ba43737e4ce Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 14 Oct 2020 11:07:56 +0200 Subject: [PATCH 2/3] Vale: Use files provided by lint staged to check --- scripts/check-docs-quality.js | 63 ++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/scripts/check-docs-quality.js b/scripts/check-docs-quality.js index 4f5f689105..cd41fa25c2 100755 --- a/scripts/check-docs-quality.js +++ b/scripts/check-docs-quality.js @@ -1,4 +1,3 @@ -#!/usr/bin/env node /* * Copyright 2020 Spotify AB * @@ -15,67 +14,71 @@ * limitations under the License. */ const { execSync, spawnSync } = require('child_process'); -const path = require('path'); // eslint-disable-next-line import/no-extraneous-dependencies const commandExists = require('command-exists'); -const listFilesTrackedByGit = 'git ls-files'; - const inheritStdIo = { stdio: 'inherit', }; const LINT_SKIPPED_MESSAGE = 'Skipping documentation quality check (vale not found). Install vale linter (https://docs.errata.ai/vale/install) to enable.\n'; -const ERROR_MESSAGE = - 'Language linter (vale) generated errors. Please check the errors and review any markdown files that you changed.\n'; +const ERROR_MESSAGE = `Language linter (vale) generated errors. Please check the errors and review any markdown files that you changed. + Possibly update .github/styles/vocab.txt to add new valid words.\n`; + +// Note: Make sure the script is run as `node check-docs-quality.js [FILES]` instead of `./check-docs-quality.js [FILES]` +// If the script receives arguments (file paths), the script is run exclusively on them. (e.g. when run via pre-commit hook) +const getFilesToLint = () => { + // Files have been provided as arguments + if (process.argv.length > 2) { + return process.argv.slice(2); + } + + let command = `git ls-files | ./node_modules/.bin/shx grep ".md"`; + if (process.platform === 'win32') { + command = `git ls-files | .\\node_modules\\.bin\\shx grep ".md"`; + } + + return execSync(command, { + stdio: ['ignore', 'pipe', 'inherit'], + }) + .toString() + .split('\n'); +}; // Proceed with the script only if Vale linter is installed. Limit the friction and surprises caused by the script. commandExists('vale') .catch(() => { console.log(LINT_SKIPPED_MESSAGE); - process.exit(0); + // process.exit(0); + process.exit(1); }) .then(() => { + const filesToLint = getFilesToLint(); + console.log('files to lint'); + console.log(filesToLint); + // xargs is not supported by shx. if (process.platform === 'win32') { - const validMDFilesCommand = `${listFilesTrackedByGit} | .\\node_modules\\.bin\\shx grep ".md"`; try { - // get list of all md files except in directories of gitignore. - let filesToLint = execSync(validMDFilesCommand, { - stdio: ['ignore', 'pipe', 'inherit'], - }); - - // set all file(s) path as absolute path - filesToLint = filesToLint - .toString() - .split('\n') - .map(filepath => - filepath ? path.join(process.cwd(), filepath) : null, - ) - .filter(Boolean); - const output = spawnSync('vale', filesToLint, inheritStdIo); // if the command does not succeed if (output.status !== 0) { // if it contains system level error. [in this case vale does not exist] if (output.error) { - console.error(ERROR_MESSAGE); + console.log(ERROR_MESSAGE); } process.exit(1); } } catch (e) { - console.error(e.message); + console.log(e.message); process.exit(1); } } else { - const validMDFilesCommand = `${listFilesTrackedByGit} | ./node_modules/.bin/shx grep ".md"`; - // use xargs - try { - execSync(`${validMDFilesCommand} | xargs vale`, inheritStdIo); - } catch (e) { - console.error(ERROR_MESSAGE); + const output = spawnSync('vale', filesToLint, inheritStdIo); + if (output.status !== 0) { + console.log(ERROR_MESSAGE); process.exit(1); } } From c1d6bc03f6ac7722987fcde48a8e89359149260f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 14 Oct 2020 11:16:23 +0200 Subject: [PATCH 3/3] Vale: Enforce vale to be run on CI --- scripts/check-docs-quality.js | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/scripts/check-docs-quality.js b/scripts/check-docs-quality.js index cd41fa25c2..f29463939b 100755 --- a/scripts/check-docs-quality.js +++ b/scripts/check-docs-quality.js @@ -23,8 +23,9 @@ const inheritStdIo = { const LINT_SKIPPED_MESSAGE = 'Skipping documentation quality check (vale not found). Install vale linter (https://docs.errata.ai/vale/install) to enable.\n'; -const ERROR_MESSAGE = `Language linter (vale) generated errors. Please check the errors and review any markdown files that you changed. +const LINT_ERROR_MESSAGE = `Language linter (vale) generated errors. Please check the errors and review any markdown files that you changed. Possibly update .github/styles/vocab.txt to add new valid words.\n`; +const VALE_NOT_FOUND_MESSAGE = `Language linter (vale) was not found. Please install vale linter (https://docs.errata.ai/vale/install).\n`; // Note: Make sure the script is run as `node check-docs-quality.js [FILES]` instead of `./check-docs-quality.js [FILES]` // If the script receives arguments (file paths), the script is run exclusively on them. (e.g. when run via pre-commit hook) @@ -47,27 +48,29 @@ const getFilesToLint = () => { }; // Proceed with the script only if Vale linter is installed. Limit the friction and surprises caused by the script. +// On CI, we want to ensure vale linter is run. commandExists('vale') .catch(() => { + if (process.env.CI) { + console.log(VALE_NOT_FOUND_MESSAGE); + process.exit(1); + } console.log(LINT_SKIPPED_MESSAGE); - // process.exit(0); - process.exit(1); + process.exit(0); }) .then(() => { const filesToLint = getFilesToLint(); - console.log('files to lint'); - console.log(filesToLint); - // xargs is not supported by shx. if (process.platform === 'win32') { + // Windows try { const output = spawnSync('vale', filesToLint, inheritStdIo); - // if the command does not succeed + // If the command does not succeed if (output.status !== 0) { - // if it contains system level error. [in this case vale does not exist] + // If it contains system level error. In this case vale does not exist. if (output.error) { - console.log(ERROR_MESSAGE); + console.log(LINT_ERROR_MESSAGE); } process.exit(1); } @@ -76,9 +79,10 @@ commandExists('vale') process.exit(1); } } else { + // Unix const output = spawnSync('vale', filesToLint, inheritStdIo); if (output.status !== 0) { - console.log(ERROR_MESSAGE); + console.log(LINT_ERROR_MESSAGE); process.exit(1); } }