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

Add link checking to docs #1242

Open
aaronmondal opened this issue Aug 7, 2024 · 5 comments · Fixed by #1256
Open

Add link checking to docs #1242

aaronmondal opened this issue Aug 7, 2024 · 5 comments · Fixed by #1256
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Comments

@aaronmondal
Copy link
Member

The current documentation might produce dangling links. Astro and starlight have integrations to automatically check the sanity of links. We should implement this so that we have a systematic way to prevent 404s.

@aaronmondal aaronmondal added bug Something isn't working enhancement New feature or request good first issue Good for newcomers labels Aug 7, 2024
@garikAsplund
Copy link

Always a good idea!

I noticed that the Starlight links validator is only for internal links. I'm guessing you want better coverage than that since there are plenty of external links, too. Also, relative links aren't supported in the plugin, though there is an option for ignoring them.

Screenshot 2024-08-07 at 2 34 46 PM

Perhaps adding in Lychee is more fitting? I'm just not sure how best to add that given you can use it in multiple ways.

@aaronmondal
Copy link
Member Author

@garikAsplund Thanks for the analysis!

Lychee indeed seems like a compelling option and matches well with our strong desire to use rust-based tooling wherever possible.

The challenge here is probably that we can't easily run it via e.g. pre-commit hooks. It would be trivial to add it to the pre-commit.nix file, but the docs tracked in this repository don't contain all contents. Some of it is generated on the fly during deployment. Maybe it would make sense to add it somewhere around the check script here:

"check": "biome ci . && astro check",

The build script first generates the files that are not tracked in the repo and then runs the checks. So if Lychee ran in that step I'd expect it to have full coverage and make deployments fail early if links are broken.

cc @blakehatch @SchahinRohani thoughs?

@aleksdmladenovic
Copy link
Contributor

aleksdmladenovic commented Aug 8, 2024

FYI, I already caught 404 issues many times while I was navigating the NativeLink Documentation Page.

They were all caused when we click the relative links in pages.

For instance, in this page - https://docs.nativelink.com/contribute/guidelines/
there's a wrong link for flake.nix.

But the correct link should be this.

The reason above is because these pages include relative links from *.md files, which are then converted to *.mdx files.

Since it's affecting actual user experience, my opinion is to fix the relative URLs first ( which are actually producing 404 errors ) and then think about adding CIs for checking 404s later on.

I can make a PR for fixing relative links in *.md files right now if you permit.
Thanks.

cc: @MarcusSorealheis, @allada , @aaronmondal

@blakehatch
Copy link
Member

@aleksdmladenovic would be incredible if you want to make a PR for this! Will promptly review it once it's open :)

@SchahinRohani
Copy link
Contributor

FYI, I think the Lychee solution is a great idea for ensuring link integrity. I agree that the current 404 errors are an issue and need to be addressed. However, I believe the relative paths should be retained in the documentation, as they are essential during development.

By properly configuring the relative paths, we can avoid the need to constantly switch between the development docs and the "hardcoded" production docs. This would significantly improve workflow and ensure consistency during development.

Therefore, I would suggest only adding hardcoded links for those that point to actual GitHub files, while keeping the relative paths for the actual Astro documentation.

cc: @aleksdmladenovic @aaronmondal @blakehatch

@allada allada reopened this Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants