Skip to content
This repository has been archived by the owner on Apr 13, 2020. It is now read-only.

[HOUSEKEEPING] Finding broken links in md files and fix them #432

Merged
merged 11 commits into from
Mar 24, 2020

Conversation

dennisseah
Copy link
Collaborator

@dennisseah dennisseah commented Mar 22, 2020

  1. created a simple tool to find broken links. ts-node tools/brokenLinks.ts
  2. fixed some eslint issues in other tools
  3. fixed the broken links. Cannot figure out this one though. https://github.com/CatalystCode/spk/blob/master/guides/infra/spk-terragrunt-overview.md, we have a reference to https://github.com/CatalystCode/spk/blob/master/SPK-Infra.md

closes microsoft/bedrock#1214

Copy link
Collaborator

@andrebriggs andrebriggs left a comment

Choose a reason for hiding this comment

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

Thank you @dennisseah for doing this

tools/brokenLinks.ts Outdated Show resolved Hide resolved
@bnookala
Copy link
Member

I did a bit of searching and found a few existing packages/tools that do similar tasks - I would err on the side of using them.

https://github.com/tcort/markdown-link-check - this one integrates with github actions, so can be run in CI as well: https://github.com/marketplace/actions/markdown-link-check

There's also this plugin for remark: https://github.com/remarkjs/remark-validate-links

Copy link
Member

@bnookala bnookala left a comment

Choose a reason for hiding this comment

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

let's try to use this tool instead, since it's being actively maintained and might fit our use cases: https://github.com/tcort/markdown-link-check

@dennisseah
Copy link
Collaborator Author

let's try to use this tool instead, since it's being actively maintained and might fit our use cases: https://github.com/tcort/markdown-link-check

ok. I will remove my tool. thanks

@dennisseah dennisseah requested a review from bnookala March 23, 2020 20:17
tools/mdLint.ts Outdated
}
};

(async (): Promise<void> => {
Copy link
Member

@bnookala bnookala Mar 23, 2020

Choose a reason for hiding this comment

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

I think this script file may be un-necessary:

See: https://github.com/tcort/markdown-link-check#check-links-from-a-local-markdown-folder-recursive

You can inline this into a package.json script instead of calling this custom script

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i cannot because i need a status code 1 to stop the script. :-)
the inline approach always return 0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry Dennis, not quite sure I understand what you're saying. Npm scripts will exit with 1 if if any of the commands fail:

  "scripts": {
    "i-will-fail": "false && true"
  },
npm run i-will-fail # will exit with code 1

are you thinking othewise?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe you can try it yourself and let me know what you get
add this to your package.json

  "md-lint": "find guides -name \\*.md -exec markdown-link-check {} \\",

or just ran (at the root of spk repo)

find guides -name \*.md -exec markdown-link-check {} \;

then

echo $?

there are errors reported by the tool and it is returning 0 status code.

Copy link
Member

@bnookala bnookala Mar 23, 2020

Choose a reason for hiding this comment

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

@dennisseah try the suggestion here: tcort/markdown-link-check#57 (comment)

find guides -name \*.md | xargs -n 1 markdown-link-check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bnookala
and what about people using windows?

Copy link
Member

Choose a reason for hiding this comment

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

@dennisseah this isn't a run-time requirement - just a development requirement. I expect that developers who are running windows who also want to contribute to spk development should do their work in a WSL environment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok then. We do not support windows runtime.

@dennisseah dennisseah merged commit f6816a5 into master Mar 24, 2020
@dennisseah dennisseah deleted the brokenLinks branch March 24, 2020 01:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken Links in md files
7 participants