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

Fix all broken links and add link checker #108

Merged
merged 9 commits into from
Nov 12, 2019

Conversation

bravegrape
Copy link

@bravegrape bravegrape commented Nov 11, 2019

Context

As a result of the change in docs structure (#97), a number of links broke.

Changes proposed in this pull request

Manually fix broken links and add a link checker such that the build will fail if there are any broken links. This should make sure future changes to the docs will not lead to more broken links.

The PR also copies the docs content from the tech docs gem: https://github.com/alphagov/tech-docs-gem/tree/master/docs. This content is currently copied over and updated on every build through the gem_docs helper method. The fragmented setup makes it hard to make changes that involve that content. The plan is to eventually move everything in the alphagov/tdt-documentation repo to the tech docs gem repo.

There will be a PR signposting the content move in alphagov/tech-docs-gem/docs This is the PR to add signposting for the content move: alphagov/tech-docs-gem#151

Guidance to review

Run build locally. html-proofer shouldn't find any broken links.

Andrea Szöllössi added 8 commits November 11, 2019 16:02
This is to avoid the two locations falling out of sync and to avoid 
having to raise PRs in two repos for some types of changes.
Now that we have all content in one place (previous commit), we don't 
need the gem_docs helper to bring the content in from the gem repo.
We moved the text in a previous commit, we should move the screenshots 
for the text, too.
There's no reason to have separate folders for 'screenshots' and 
'diagrams'.
bravegrape pushed a commit to alphagov/tech-docs-gem that referenced this pull request Nov 11, 2019
@bravegrape bravegrape marked this pull request as ready for review November 11, 2019 17:50
@bravegrape bravegrape mentioned this pull request Nov 11, 2019
@36degrees
Copy link
Contributor

A very minor (and existing) thing that'd be good to fix at some point but shouldn't block this – there's a mix of wrapped and non-wrapped lines within and across all the files. It'd be good to try and make it consistent at some point.

@@ -7,4 +7,229 @@ review_in: 1 day

# Configuration options

<%= gem_docs 'configuration.md' %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we need to do a pull request on the gem repo to remove content?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the PR to add signposting for the content move: alphagov/tech-docs-gem#151

Copy link
Author

Choose a reason for hiding this comment

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

Yup - I've raised this PR to cover the content move: alphagov/tech-docs-gem#151

@bravegrape
Copy link
Author

A very minor (and existing) thing that'd be good to fix at some point but shouldn't block this – there's a mix of wrapped and non-wrapped lines within and across all the files. It'd be good to try and make it consistent at some point.

@36degrees I've raised an issue to keep track of this: #109

Copy link
Contributor

@jonathanglassman jonathanglassman left a comment

Choose a reason for hiding this comment

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

HTML proofer runs successfully. Good work. Do a show and tell on it!

@bravegrape bravegrape merged commit 6c40034 into master Nov 12, 2019
@bravegrape bravegrape deleted the fix-all-broken-links-forever branch November 12, 2019 17:00
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.

4 participants