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

Fail build if there are dead internal links #187

Closed
wants to merge 1 commit into from

Conversation

bjgill
Copy link

@bjgill bjgill commented Apr 4, 2019

Using html-proofer. Will apply to new projects only - existing users of the template would need to apply this change manually.

Resolves #186

Using html-proofer. Will apply to new projects only - existing users of the template would need to apply this change manually.
@bjgill
Copy link
Author

bjgill commented Apr 4, 2019

Interesting. It turns out it catches other problems as well. The build's just about to fail. Which nicely proves that this linter does successfully fail the build if it detects problems 😝

- build/index.html
  *  image /images/gov.uk_logotype_crown_invert_trans.png does not have an alt attribute (line 42)
HTML-Proofer found 1 failure!

I don't really know enough about HTML to know if this is a serious problem, however.

I guess this is possibly a question for the maintainers of the template, who might have opinions. Here is the list of all the things that html-proofer checks. What do we actually want to check? I'm tempted to say everything - it's fairly simple for users to disable individual lints that do not make sense for them. It might be easiest to just check for dead links (i.e. disable everything else), and let users re-enable anything they think they need...

Images

img elements:

  • Whether all your images have alt tags
  • Whether your internal image references are not broken
  • Whether external images are showing
  • Whether your images are HTTP

Links

a, link elements:

  • Whether your internal links are working
  • Whether your internal hash references (#linkToMe) are working
  • Whether external links are working
  • Whether your links are HTTPS
  • Whether CORS/SRI is enabled

Scripts

script elements:

  • Whether your internal script references are working
  • Whether external scripts are loading
  • Whether CORS/SRI is enabled

Favicon

  • Whether your favicons are valid.

OpenGraph

  • Whether the images and URLs in the OpenGraph metadata are valid.

HTML

  • Whether your HTML markup is valid. This is done via Nokogiri to ensure well-formed markup.

@bravegrape
Copy link

Thanks for raising this PR @bjgill 👍 We'll have a closer look at what exactly html-proofer checks for and see how to align that with the style guide and best practice.

@NickColley
Copy link

This feature looks really useful.

One thing to consider is if this feature be proposed on the tech-docs-gem? Then it could be distributed as part of that gem.

If there's no activity on this for a while we'll close this out, to tidy things up, so let us know if you want to continue working on this. 👍

bravegrape pushed a commit to alphagov/tdt-documentation that referenced this pull request Nov 4, 2019
Follow example from 
alphagov/tech-docs-template#187

Also contains gem update.

This will be useful to keep track of fixing the 24 broken links 
currently in the tdt docs.
bravegrape pushed a commit to alphagov/tdt-documentation that referenced this pull request Nov 11, 2019
@NickColley
Copy link

I think it would be good to keep trying this approach in a few more of the important tech docs instances, for example the GaaP projects.

Then, when we're confident it works across different scenarios we could consider adding this to the gem itself?

@jonathanglassman
Copy link
Contributor

I think it would be good to keep trying this approach in a few more of the important tech docs instances, for example the GaaP projects.

Then, when we're confident it works across different scenarios we could consider adding this to the gem itself?

Seems sensible.

@bravegrape
Copy link

I think it would be good to keep trying this approach in a few more of the important tech docs instances, for example the GaaP projects.

Then, when we're confident it works across different scenarios we could consider adding this to the gem itself?

Yup, sounds sensible, but worth mentioning that some docs, like the PaaS docs have implemented link checking in other ways.

Might be one to try for the Pay docs @m-green ?

@m-green
Copy link
Contributor

m-green commented Nov 13, 2019

Definitely @bravegrape - was literally just trying this out locally on Pay docs after seeing this PR :)

@lfdebrux
Copy link
Member

lfdebrux commented Oct 7, 2022

Closing this until we're ready to pick up #186 again.

@lfdebrux lfdebrux closed this Oct 7, 2022
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.

Add linter to check for dead links?
6 participants