Skip to content

tests: links: misc fixes#1067

Merged
shcheklein merged 6 commits into
masterfrom
link-check-issues
Mar 18, 2020
Merged

tests: links: misc fixes#1067
shcheklein merged 6 commits into
masterfrom
link-check-issues

Conversation

@casperdcl
Copy link
Copy Markdown
Contributor

@casperdcl casperdcl commented Mar 18, 2020

  • support Mac's version of sed
  • print file names of errors in diffs (useful in large PRs)
  • run from any dir (not just repo root)
  • fixes link checker: fix issues #1066

@shcheklein shcheklein temporarily deployed to dvc-landing-link-check-cvzfhcg March 18, 2020 00:08 Inactive
#!/usr/bin/env bash
(find pages/ content/docs/ src/ .github/ -name '*.md' -o -name '*.js' && ls *.md *.js) \
repo="$(dirname "$(realpath "$(dirname "$0")")")"
(find "$repo"/pages/ "$repo"/content/docs/ "$repo"/src/ "$repo"/.github/ -name '*.md' -o -name '*.js' && ls "$repo"/*.md "$repo"/*.js) \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does it fail if one of the directories does not exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no - tested (exit status still zero). I presume you're talking about the warning

find: ‘...dvc.org/pages/’: No such file or directory

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes, would be good to catch changes in the docs structure if it's just a matter of adding some flag

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I assume pages would be generated if the site was built. Are you saying it should be removed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no, no ... I'm not even sure if we need to test generated stuff .. just saying that next time someone decides to move /content to something like /static/content would be nice to detect that change ... again, not a big deal

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right. Sounds like something for a different issue. Though it sounds like doing something like a directory move leaved the onus on the mover to update all tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

like a directory move leaved the onus on the mover to update all tests

correct! but it's better for tests to start failing.

@shcheklein shcheklein temporarily deployed to dvc-landing-link-check-cvzfhcg March 18, 2020 00:20 Inactive
@casperdcl casperdcl self-assigned this Mar 18, 2020
@casperdcl casperdcl added 🐛 type: bug Something isn't working. website: eng-doc DEPRECATED JS engine for /doc p1-important Active priorities to deal within next sprints labels Mar 18, 2020
Comment thread scripts/exclude-links.txt
@shcheklein shcheklein temporarily deployed to dvc-landing-link-check-cvzfhcg March 18, 2020 00:40 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-link-check-cvzfhcg March 18, 2020 00:46 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-link-check-cvzfhcg March 18, 2020 00:52 Inactive
@shcheklein
Copy link
Copy Markdown
Contributor

ready to merge? :)

@shcheklein shcheklein merged commit 8148133 into master Mar 18, 2020
@shcheklein shcheklein deleted the link-check-issues branch March 27, 2020 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p1-important Active priorities to deal within next sprints 🐛 type: bug Something isn't working. website: eng-doc DEPRECATED JS engine for /doc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

link checker: fix issues

2 participants