docs
docs copied to clipboard
Update package.json scripts
- remove
build
fromlinkinator
script - switch to using
--verbosity
for linkinator
Signed-off-by: XhmikosR [email protected]
By submitting this pull request, I confirm the following: please fill any appropriate checkboxes, e.g: [X]
- [x] I have read and understood the contributors guide, as well as this entire template.
- [x] I have made only one major change in my proposed changes.
- [x] I have commented my proposed changes within the code.
- [x] I have tested my proposed changes, and have included unit tests where possible.
- [x] I am willing to help maintain this change if there are issues with it later.
- [x] I give this submission freely and claim no ownership.
- [x] It is compatible with the EUPL 1.2 license
- [x] I have squashed any insignificant commits. (
git rebase
)
Please make sure you Sign Off all commits. Pi-hole enforces the DCO.
✔️ Deploy Preview for pihole-docs ready!
🔨 Explore the source changes: dd16f45e0ca14067a62c472d80df2353d353f641
🔍 Inspect the deploy log: https://app.netlify.com/sites/pihole-docs/deploys/60fd817b58a3f00007646729
😎 Browse the preview: https://deploy-preview-543--pihole-docs.netlify.app
I disagree with the first point
- remove
build
fromlinkinator
script
We've added this after two or three cases where users ran npm test
without any issues because they forgot to run build
as a separate step. However, the CI reported errors and the initial confusion was there. To me - because the linkinator
test is not meaningful without a preceding build - this is a logical addition.
This is useless because npm t
will fail if the build folder isn't present and 2, the build script runs needlessly twice on CI.
Proof FYI:
C:\Users\xmr\Desktop\docs>npm t
> [email protected] test C:\Users\xmr\Desktop\docs
> npm run markdownlint && npm run linkinator
> [email protected] markdownlint C:\Users\xmr\Desktop\docs
> markdownlint-cli2 "**/*.md" "!**/node_modules/**"
markdownlint-cli2 v0.1.3 (markdownlint v0.23.1)
Finding: **/*.md !**/node_modules/**
Linting: 73 file(s)
Summary: 0 error(s)
> [email protected] linkinator C:\Users\xmr\Desktop\docs
> linkinator site --recurse --silent --skip "^(?!http://localhost)"
🏊♂️ crawling site
Error: The provided glob "site" returned 0 results. The current working directory is "C:\Users\xmr\Desktop\docs".
at Object.processOptions (C:\Users\xmr\Desktop\docs\node_modules\linkinator\build\src\options.js:59:23)
at async LinkChecker.check (C:\Users\xmr\Desktop\docs\node_modules\linkinator\build\src\index.js:32:25)
at async main (C:\Users\xmr\Desktop\docs\node_modules\linkinator\build\src\cli.js:162:20)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
If you insist on having it there, I could probably refactor the scripts and update the CI script. But it's not that hard for people to obviously build the site beforehand...
Yes, but if it has been built before the test would be running on an outdated version.
It may not be hard, but remember, we need to make the barrier to entry to be as absolutely low as possible. If we have to wait longer for CI to run then that's the cost of how we operate.
People editing these docs and files likely have not ever used any of these tools before and may be using the GitHub interface to make their edits.
I understand the desire to have a lean and mean build process but I think we need to error on the side of caution and be mindful of who our submitters may be.
I'll refactor the scripts later and add a start
and a dev
script which will watch for changes and should simplify the whole process.
I appreciate it, thanks!
So, this is a lot trickier than I thought :/
mkdocs serve
doesn't write the files to the disk (my guess is so that it's faster when developing) so we can't use it, but we should use it.
The other solution would be to have a watch script which would call the npm test
script after the mkdocs build
but this is definitely a worse developing behavior since it's a lot slower.
Another solution would be to have the build script as a pretest
script; this would automatically build when one runs npm test
.
Let me know your thoughts.
Thank you for taking a look at it and working on it.
I think the pretest
step would solve the issues, DL would need to confirm that works for him as well. If we properly document that npm test
is the step that needs to be done then I think we would cover the cases we need to cover.
Another solution would be to have the build script as a pretest script; this would automatically build when one runs npm test.
So what would be the difference in the end?
That the linkinator script won't affect the build and only the npm test
script will have build automatically run when it runs.