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

[DOCS] Fix multiple broken links #587

Merged
merged 1 commit into from Jun 6, 2022
Merged

[DOCS] Fix multiple broken links #587

merged 1 commit into from Jun 6, 2022

Conversation

ojacques
Copy link
Contributor

What does this PR do?

  • Fix multiple broken links in the documentation. They used to be valid when the doc was outside of mkdocs. With this PR, they are valid both ways.
  • Add missing add-ons in the navigation which were present in Add-Ons overview

Motivation

I clicked on some links which resulted in 404.

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

  • Documentation was tested locally on both mkdocs and GitHub / pure markdown with the changes.
  • All links which go to the GitHub repository point to absolute path instead of relative. This is needed when linking from an mkdocs rendered documentation

@ojacques ojacques temporarily deployed to EKS Blueprints Test May 31, 2022 13:43 Inactive
| [HashiCorp Vault](https://github.com/aws-ia/terraform-aws-eks-blueprints/blob/main/docs/add-ons/vault.md) | Deploys HashiCorp Vault into an EKS cluster.
| [VPA](https://github.com/aws-ia/terraform-aws-eks-blueprints/blob/main/docs/add-ons/vpa.md) | Deploys the Vertical Pod Autoscaler into an EKS cluster. |
| [YuniKorn](https://github.com/aws-ia/terraform-aws-eks-blueprints/blob/main/docs/add-ons/yunikorn.md) | Deploys Apache YuniKorn into an EKS cluster. |
| [Agones](agones.md) | Deploys Agones into an EKS cluster. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we want the full URL since this is coming from the mkdocs generated site pointing back to the individual addon REAMDEs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we do, as the add-ons READMEs are all in the mkdocs docs/add-ons folder. Actually, in the same folder as this index.md file.

Copy link
Contributor

Choose a reason for hiding this comment

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

The sidebar navigation pane (left) uses the local addon docs, the table links to the addon module READMEs (right/table)
image

If they both lead to the same place, then I would say we remove the table and rely on the sidebar navigation pane since it is redundant and another place we have to ensure is updated

Copy link
Contributor Author

@ojacques ojacques Jun 1, 2022

Choose a reason for hiding this comment

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

Yes, maintaining nav is a pain. Mkdocs actually complains about pages not included in nav.

I have successfully used an mkdocs plugin - awesome-pages - to totally remove this concern while keeping navigation under control.

If you prefer (and allow an additional mkdocs plugin), I can close this PR and issue another one which leverages awesome-pages plugin and maintain the same nav structure without the nav entries in mkdocs.yml.

Let me know.

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@Zvikan
Copy link
Contributor

Zvikan commented Jun 6, 2022

Manually deployed these changes to "Dev" preview version: https://aws-ia.github.io/terraform-aws-eks-blueprints/dev/ .
Moving forward will create an automation to have preview mkdocs following #596 changes.
Feel free to use the dev preview in order to test the changes.

@Zvikan Zvikan added the documentation Improvements or additions to documentation label Jun 6, 2022
@bryantbiggs bryantbiggs merged commit dcd70c4 into aws-ia:main Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants