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 option to locally run HTMLProofer #2285

Merged
merged 12 commits into from
Jan 25, 2025

Conversation

nickspaargaren
Copy link
Contributor

@nickspaargaren nickspaargaren commented Dec 31, 2024

What

This PR adds the option to locally run HTMLProofer. With the Python upgrade #2271 I ran into a problem with the html generation. This makes it easier to run the check without waiting for the Github actions to finish.

How to test

  1. Check out this branch
  2. Open VSCode and (Re)open in container. Or use: >Dev Containers: Reopen in Container
  3. Wait for the container build to finish.
  4. Use cd docs/ to navigate to the docs folder
  5. Run make html to generate the html inside docs/_build
  6. Use cd ../ to navigate back to the root folder
  7. Run the htmlproofer command used in https://github.com/writethedocs/www/blob/main/.github/workflows/ubuntu.yml
    • htmlproofer --ignore-files "/404/,/2013/,/2014/,/2015/,/2016/,/2017/,/search\/index.html/" --allow-hash-href=true --enforce-https=false --ignore-missing-alt=true --disable-external=true docs/_build/html
  8. Make sure the command can be run and checks for html warnings.

TODO

  • Make sure Github actions pass with new ruby/setup-ruby action.

📚 Documentation preview 📚: https://writethedocs-www--2285.org.readthedocs.build/

@plaindocs
Copy link
Contributor

I'd be open to moving the htmlproofer link check out into external config or script to make it easier to run both locally and in CI.

The issue I ran into when running locally was that there are a lot of links like "/site-map/" which fail locally, and we'd have to figure out a way to address those. Or run it against a locally running server. 🤷‍♂️ Also a load of options which are valid in CI aren't locally (also probably a different version thing).

This part of the tool chain is really not a joy.

@nickspaargaren
Copy link
Contributor Author

I'd be open to moving the htmlproofer link check out into external config or script to make it easier to run both locally and in CI.

The issue I ran into when running locally was that there are a lot of links like "/site-map/" which fail locally, and we'd have to figure out a way to address those. Or run it against a locally running server. 🤷‍♂️ Also a load of options which are valid in CI aren't locally (also probably a different version thing).

This part of the tool chain is really not a joy.

I saw this also locally. Maybe this is something we can fix later if the sphinx-ubuntu passes?

My secondary goal is to keep the htmlproofer updated with dependabot as well. At first i thought it was a bug that was fixed in a newer version of htmlproofer.

@nickspaargaren nickspaargaren marked this pull request as ready for review January 20, 2025 12:41
Copy link
Contributor

@plaindocs plaindocs left a comment

Choose a reason for hiding this comment

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

I've not tested the local side, but the remote looks like it's working.

Do you want to check in a broken link to make sure it crashes out, then we can revert and merge?

@nickspaargaren
Copy link
Contributor Author

I've not tested the local side, but the remote looks like it's working.

Do you want to check in a broken link to make sure it crashes out, then we can revert and merge?

Sure thing, let me know if you've seen the failed pipeline so I can revert the temporary commit!

Copy link
Contributor

@plaindocs plaindocs left a comment

Choose a reason for hiding this comment

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

Lovely. 👍

@nickspaargaren nickspaargaren force-pushed the locally-run-HTMLProofer branch from c8ab172 to d5090b8 Compare January 21, 2025 06:35
@nickspaargaren nickspaargaren force-pushed the locally-run-HTMLProofer branch from d5090b8 to 0d358f7 Compare January 21, 2025 06:37
@nickspaargaren
Copy link
Contributor Author

Lovely. 👍

Temporary commit has been removed, ready to merge!

@plaindocs plaindocs enabled auto-merge (squash) January 22, 2025 02:16
@plaindocs plaindocs merged commit 163afe8 into writethedocs:main Jan 25, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants