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

Added automatic markdown link checking and fixed broken links #1970

Merged
merged 7 commits into from
Nov 2, 2022

Conversation

Jimmys20
Copy link
Contributor

@Jimmys20 Jimmys20 commented Oct 30, 2022

Fixes #1969

This PR enables automatic markdown link checking using markdown-link-check utility. I fixed most of the broken links and I created an ignore list for the links I wasn't sure how to fix.

Microsoft Reviewers: Open in CodeFlow

Copy link
Member

@krwq krwq left a comment

Choose a reason for hiding this comment

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

LGTM, +1 if you make our build break whenever there is any broken link. Possibly markdownlint has option for that: https://github.com/dotnet/iot/blob/main/.vsts-ci.yml#L107

@krwq
Copy link
Member

krwq commented Oct 31, 2022

@krwq
Copy link
Member

krwq commented Oct 31, 2022

You can use something similar to this to run it on all files:

ls -r *.md | %{markdown-link-check $_.FullName; if (-Not $?) { throw "One of the links is wrong" } }

There are plenty of other dead links in the repo.

Probably some configuration will be needed as well to ignore some of the bad links (some require authentication)

@Jimmys20 Jimmys20 marked this pull request as draft November 1, 2022 09:06
@Jimmys20
Copy link
Contributor Author

Jimmys20 commented Nov 1, 2022

This is the list of the problematic links according to markdown-link-check that I wasn't sure how to fix:

broken-links.txt

Certain paths can be ignored by adding the appropriate pattern to .mlc-config.json e.g.:

{
  "ignorePatterns": [
    {
      "pattern": "^http://example.net"
    }
  ]
}

@Jimmys20 Jimmys20 marked this pull request as ready for review November 1, 2022 16:58
@krwq
Copy link
Member

krwq commented Nov 2, 2022

@Jimmys20 I'll suggest create that config file and add each link with missing datasheet or similar (outside of our repo) individually and fix it separately outside of this PR. Once you're done let's open issue and point to that ignore file that those datasheets are missing and let's figure out path forward (possibly we should be copying those datasheets into our repo, not sure but we should figure out generic solution). In this PR let's focus on enabling the automatic checks and make sure links within the repo are correct.

If you can find alternative link to datasheet that's fine too 😄 But I think we should open issue anyway because eventually another external link will go 404 and we'll be in the same spot so we should figure out what to do in those situations

.mlc-config.json Outdated
@@ -0,0 +1,4 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

I'll suggest to match naming with: https://github.com/dotnet/iot/blob/main/.markdownlint.json

so something like .markdown-link-checker.json

@krwq
Copy link
Member

krwq commented Nov 2, 2022

Should the CI be showing link checker errors? I'm not seeing any (or did you fix all of the issues 👀) Possibly it will show only after merging

@Jimmys20
Copy link
Contributor Author

Jimmys20 commented Nov 2, 2022

The last broken links within repo are in https://github.com/dotnet/iot/blob/main/samples/bmp280-sensor-azure-iot-hub/Readme.md: ../.vscode/launch.json and ../.vscode/tasks.json. Any suggestion for those?

@Jimmys20 Jimmys20 changed the title Changed src/devices/Mcp3008 to src/devices/Mcp3xxx Added automatic markdown link checking and fixed broken links Nov 2, 2022
Copy link
Member

@krwq krwq left a comment

Choose a reason for hiding this comment

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

File an issue on the patterns we had to add to suppress errors and assuming there is no more errors LGTM and thank you for the effort!

@krwq krwq merged commit d47bf98 into dotnet:main Nov 2, 2022
@krwq
Copy link
Member

krwq commented Nov 2, 2022

Thanks again @Jimmys20! I'll file an issue shortly

@Jimmys20 Jimmys20 deleted the fix-broken-links branch November 2, 2022 23:28
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
This pull request was closed.
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 link
3 participants