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

feat: TechDocs - Add vale linter to check words quality in md files. #2631

Merged
merged 19 commits into from Oct 2, 2020

Conversation

Marvin9
Copy link
Contributor

@Marvin9 Marvin9 commented Sep 28, 2020

Hey, I just made a Pull Request!

Closes #2031

Approach to solve #2031 using vale linter.

  1. Added basic vale setup file .vale.ini. source
  2. vale does not provide functionality to ignore folders, we have to provide files manually vale foo.md bar.md to run linter. So automatic way to achieve it is by grep & git.
  • list all files which are tracked by git. (node_modules and other unnecessary directories are ignored).
  • find md files.
  • pass above files to vale
git ls-files | grep -h ".md" | xargs vale
  • added above command in package.json as lint:docs.
  1. Generated vocab.txt file.
  • Some keywords, names (eg. api, https) are considered as vale.Spelling error, so we have to add them inside vocab.txt file.
  • We can list all unique words which are considered as error in vocab.txt using below command.
yarn run lint:docs | grep -o "'[a-z A-Z]*'" | grep -o "[a-z A-Z]*" | sort | uniq > .github/styles/vocab.txt
  1. Now we have to manually validate all words listed in vocab.txt, discard which are invalid and correct them in file where they were located.
  2. Added vale github actions.
  • we have to add that action before node_modules are added.

Above steps are included in this PR.

Usage:

yarn run lint:docs

Limitation:

  • update in vocab.txt is required for false positives.

✔️ Checklist

  • All tests are passing yarn test
  • Screenshots attached (for UI changes)
  • Relevant documentation updated
  • Prettier run on changed files
  • Tests added for new functionality
  • Regression tests added for bug fixes

Initialize .vale.ini file
Add 'lint:docs' script to package.json, to lint all md files except the ones which are located in node_modules
Generate 'vocab.txt' by using command 'yarn run lint:docs' | grep -o ''[a-z A-Z]*'' | grep -o '[a-z A-Z]*' | sort | uniq > .github/styles/vocab.txt
Add steps to github workflow 'master' to check docs quality
@Marvin9 Marvin9 requested review from a team as code owners September 28, 2020 08:40
@Marvin9 Marvin9 changed the title Feat/docs linter feat: Add vale linter to check words quality in md files. Sep 28, 2020
@emmaindal
Copy link
Member

emmaindal commented Sep 28, 2020

WOW!! This is awesome, will definitely look through it more asap 💯

@OrkoHunter OrkoHunter changed the title feat: Add vale linter to check words quality in md files. feat: TechDocs - Add vale linter to check words quality in md files. Sep 28, 2020
@OrkoHunter OrkoHunter added the area:techdocs Related to the TechDocs Project Area label Sep 28, 2020
Copy link
Collaborator

@andrewthauer andrewthauer left a comment

Choose a reason for hiding this comment

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

This is really neat. I looked at vale a little while back but haven't tried it. Good to see it in use.

An additional thought would be to have a lint-staged / git hook check as well. This would help avoid people not noticing issues until CI.

package.json Outdated Show resolved Hide resolved
@Marvin9
Copy link
Contributor Author

Marvin9 commented Sep 28, 2020

Updates:

  • separate CI to check docs quality. All files are checked on push and only updated filese are checked on pull_request
  • used shx npm package to wrap grep command.
  • xargs is not supported in shx, to solve this issue on windows platform, I've tried to achieve similar functionality in node script at /scripts/check-docs-quality.js
  • Added command in pre-commit hook.

Can you please check yarn run lint:docs on windows machine
@emmaindal @andrewthauer

@hooloovooo
Copy link
Contributor

Updates:

* separate CI to check docs quality. All files are checked on `push` and only updated filese are checked on `pull_request`

* used [shx](https://github.com/shelljs/shx) npm package to wrap `grep` command.

* `xargs` is not supported in `shx`, to solve this issue on windows platform, I've tried to achieve similar functionality in node script at `/scripts/check-docs-quality.js`

* Added command in pre-commit hook.

Can you please check yarn run lint:docs on windows machine
@emmaindal @andrewthauer

Hey! This is the error i'm getting trying to run on windows.

PS C:\Users\sqvar\projects\backstage> yarn run lint:docs
yarn run v1.22.1
$ node ./scripts/check-docs-quality
The command line is too long.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@OrkoHunter OrkoHunter moved this from Incoming to Awaiting Review/Review In Progress in [DEPRECATED] Hello docs-like-code Sep 28, 2020
@Marvin9
Copy link
Contributor Author

Marvin9 commented Sep 28, 2020

@hooloovooo Can you please check it again?

package.json Outdated Show resolved Hide resolved
@hooloovooo
Copy link
Contributor

@hooloovooo Can you please check it again?

Now i'm getting this error when running the lint command
2020-09-29-103155_707x380_scrot

@hooloovooo
Copy link
Contributor

@hooloovooo Can you please check it again?

If running check-docs-quality.js directly using node I get this error instead

'.' is not recognized as an internal or external command,
operable program or batch file.

@Marvin9
Copy link
Contributor Author

Marvin9 commented Sep 29, 2020

@andrewthauer I have removed shebang as we are using this script only once. Is it ok?

@hooloovooo This time I have checked in windows machine and is working. Can you please verify?

@andrewthauer
Copy link
Collaborator

@Marvin9 - I'm a bit indifferent, but my default is to use executables for scripts. I would say that if the file has an executable flag the the shebang should be there if not it's pointless.

@hooloovooo
Copy link
Contributor

@Marvin9 Confirmed to work now! :)

@Marvin9
Copy link
Contributor Author

Marvin9 commented Sep 30, 2020

Documentation updates required to mention installation of vale cli. I will update that.

@Marvin9 Marvin9 requested a review from emmaindal October 1, 2020 04:31
@emmaindal
Copy link
Member

@Marvin9 Looks good to me, there is just a markdown files quality check that needs to pass before merge!

@Marvin9 Marvin9 requested a review from a team as a code owner October 1, 2020 08:48
@emmaindal
Copy link
Member

@spotify/silver-lining can we get a review from you since this PR makes some spelling changes to your documentation!

Copy link
Contributor

@brendasukh brendasukh left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@emmaindal emmaindal merged commit 5c6a035 into backstage:master Oct 2, 2020
[DEPRECATED] Hello docs-like-code automation moved this from Awaiting Review/Review In Progress to Done Oct 2, 2020
@emmaindal
Copy link
Member

@Marvin9 thank you so much for your contribution 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:techdocs Related to the TechDocs Project Area
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

TechDocs: As a producer of doc sites, I want the Vale Linter to be adopted for doc quality checks
7 participants