Skip to content

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 31, 2020

There's a bug in the "jekyll-relative-links" plugin that causes wrapped links to not work. Also replacing some links to point to the .md file, so that IDE's can check if the anchors are valid.
Finally, replaced some links to point to their new locations, so that users don't get redirected..

Found wrapped markdown links by grepping for \[[^\]]*\n.*\]\([^\n]+.md

fixes #10546
closes #10551

@thaJeztah
Copy link
Member Author

ping @usha-mandya PTAL

@thaJeztah thaJeztah force-pushed the fix_markdown_links branch from 45395dd to f53fde8 Compare April 1, 2020 06:55
usha-mandya
usha-mandya previously approved these changes Apr 1, 2020
Copy link
Member

@usha-mandya usha-mandya left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for fixing all the links :)

usha-mandya
usha-mandya previously approved these changes Apr 1, 2020
there's a bug causing wrapped links to not work, and replacing
some links to point to the .md file, so that IDE's can check
if the anchors are valid. Also replaced some links to point
to their new location.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- [Discover Network Encryption on
Kubernetes](/ee/ucp/kubernetes/kubernetes-network-encryption/)
- [Deploy an Ingress Controller on Kubernetes](/ee/ucp/kubernetes/layer-7-routing/)
- [Discover Network Encryption on Kubernetes](../kubernetes-network-encryption.md)
Copy link
Member

Choose a reason for hiding this comment

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

Wondering why we have a relative path and an absolute path for topics in the same directory

Copy link
Member Author

Choose a reason for hiding this comment

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

Looked at some of those (probably worth a discussion after ee-topics have migrated). So:

Advantage of relative links:

  • they work when browsing the markdown on GitHub (I think absolute links don't work there, but would have to double-check)
  • moving a whole directory to a different location doesn't break links to pages in the same directory
  • ^^ this could help with, e.g. moving whole of ee/ucp topic to an archived location (datacenter/ucp/3.x), and still linking to related topics for the same version (not the "current" version)

Disadvantage of relative links:

  • they can be harder to read. it's "ok" when linking to a child directory (child-dir/page.md), but becomes hard to follow when linking to parent directories (../../../somedir/page.md)
  • it's hard to find/replace links when things move (can't search for /some/old/dir/page.md to find which links needs updating)

I think the disadvantages are real; mostly when linking to "other" topics (linking to, e.g., a reference-docs page from a product manual). I'm ok with using relative links to link within the same topic though.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, thanks for that. Agree, relative links can be very confusing when linking to parent directories. We can discuss this in one of our weekly syncs.

There's a bug in the "jekyll-relative-links" plugin that causes wrapped links to not work.
Also replacing some links to point to the .md file, so that IDE's can check if the anchors
are valid. Finally, replaced some links to point to their new locations, so that users don't
get redirected..

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fix_markdown_links branch from 6622697 to 7640687 Compare April 1, 2020 10:54
@usha-mandya usha-mandya merged commit 331554f into docker:master Apr 1, 2020
@thaJeztah thaJeztah deleted the fix_markdown_links branch April 1, 2020 11:09
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.

broken doc link from start-the-docker-daemon to configure-docker-to-start-on-boot

2 participants