Fix broken links in documentation #6305

arielshaqed · 2023-08-02T08:26:23Z

Also make them relative, which relies less on Jekyll-time magic and
therefore easier to check while working locally.

Fixes #6294.

github-actions · 2023-08-02T08:27:14Z

🎊 PR Preview bb00c91 has been successfully built and deployed to https://treeverse-lakeFS-preview-pr-6305.surge.sh

🕐 Build time: 0.014s

_{🤖 By surge-preview}

That one understands `.md` suffixes, and is arguably nicer to use. Absolute links in aws.md

arielshaqed · 2023-08-03T06:56:33Z

Hiyush reviewers...

How to review

Sorry for the too-large PR. Unfortunately #6294 is the tip of the iceberg (and I don't mean the table format). Many of our links were broken, for multiple reasons. The main change is to change the instructions for how to link -- please start by reviewing that one! After that, many links were simply broken -- not only the ones referenced in the issue. I don't know why our "check links" check failed to find them; the fact of the matter is that it did not. Additionally, many of our anchored links were also broken; I fixed all of those that I could find.

What to review

Most of the changes follow one of these three mechanics:

{{ site.baseurl }}/path/to/page.html ==> {% link path/to/page.md %}.
But... {% link ... %} doesn't do redirects, it checks the link at the Jekyll level, so {{ site.baseurl }}/path/to/directory/ ==> {% link path/to/directory/index.md %}.
And also... there is no point in linking to .html when the page is called .md. This is not a redirect, Jekyll will generate a page with the extension .html. But its link-checking still needs the correct name.
I lied, there are actually four things: {% link ... %} doesn't work with anchors because Jekyll and moustache syntax something. The workaround that I found was to use a link at the bottom of the text - that lets Jekyll do its thing and only then Markdown does its thing for the link, so anchors suddenly work.

I did not fix any link names in (non-Markdown...) assets. There is no risk of breakage there: no anchors, no extensions change. I believe that changing these would increase risk of breakage.

Some of the changes reflect broken links that the Jekyll {% link ... %} was able to find.

Why bother

The existing docs contain multiple breakages, and it seems that the Surge-based link checking was unable to find them. It does find other breakages, that Jekyll does not find, so best to keep both. {% link ... %} is syntactic and can check some links, {{site.baseurl}} is a macro and prevents Jekyll from checking. As always, the sooner we can check links the better.

johnnyaug · 2023-08-03T07:04:32Z

.github/workflows/docs-pr.yaml

@@ -6,6 +6,9 @@ on:
      - "docs/**"
    branches:
      - master
+  push:
+    paths:
+      - "docs/**"


Do we want this?

I think we do. So I'll move it to another PR and ask you and @nopcoder to TAL.

Removed this, opened #6311.

johnnyaug

Marvelous!

arielshaqed

!

arielshaqed · 2023-08-03T07:24:23Z

.github/workflows/docs-pr.yaml

@@ -6,6 +6,9 @@ on:
      - "docs/**"
    branches:
      - master
+  push:
+    paths:
+      - "docs/**"


I think we do. So I'll move it to another PR and ask you and @nopcoder to TAL.

The `.md` -> `.html` translation doesn't appear to work with `#` anchors. But... if we use an unsuffixed page name then _no_ translation is needed, and it might work!

It seems from [Jekyll docs](https://jekyllrb.com/docs/liquid/tags/#linking-to-posts) that there is no way to add anchors to link tags _inside_ a Markdown link. Instead, use references at the end of the file, where they seem to work.

Don't bother with images and other assets, though -- they are actually _easier_ to map because their extensions never change etc.

reference/{authorization => rbac}

Uggh.

This makes Surge try harder and run more often. Discovered while I was working on #6305, where it ran once after creating the PR but then stopped. Admittedly GitHub was not particularly responsive to running actions at the time.

arielshaqed · 2023-08-03T07:30:17Z

THANKS!

arielshaqed marked this pull request as draft August 2, 2023 08:26

arielshaqed added bug Something isn't working docs Improvements or additions to documentation labels Aug 2, 2023

arielshaqed added exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached labels Aug 2, 2023

arielshaqed force-pushed the docs/6294-fix-broken-links branch 3 times, most recently from 7b87ab4 to b6fc43a Compare August 2, 2023 13:09

Fix links to use Jekyll %link syntax

2ec1ae2

That one understands `.md` suffixes, and is arguably nicer to use. Absolute links in aws.md

arielshaqed force-pushed the docs/6294-fix-broken-links branch 13 times, most recently from 851c104 to e52dbce Compare August 3, 2023 06:20

arielshaqed marked this pull request as ready for review August 3, 2023 06:34

arielshaqed requested a review from johnnyaug August 3, 2023 06:34

johnnyaug reviewed Aug 3, 2023

View reviewed changes

johnnyaug approved these changes Aug 3, 2023

View reviewed changes

arielshaqed commented Aug 3, 2023

View reviewed changes

Try to use unsuffixed page name when adding an anchor

bd4f000

The `.md` -> `.html` translation doesn't appear to work with `#` anchors. But... if we use an unsuffixed page name then _no_ translation is needed, and it might work!

arielshaqed added 11 commits August 3, 2023 10:25

Fix anchors to use references at end of file

a0574bf

It seems from [Jekyll docs](https://jekyllrb.com/docs/liquid/tags/#linking-to-posts) that there is no way to add anchors to link tags _inside_ a Markdown link. Instead, use references at the end of the file, where they seem to work.

Fix moar links

6832456

Try to fix download link

004d73b

Fix links in azure.md

beae2da

Update how to create links in docs.

35212d8

Translate *ALL* links to use {% link ... %}

1911c73

Don't bother with images and other assets, though -- they are actually _easier_ to map because their extensions never change etc.

Try to bump security_update.{html => md}

39114b5

Broken links in glossary

fcd5d48

reference/{authorization => rbac}

Fix more links

ab61475

%link => % link

f517f73

Uggh.

Revert docs/reference.md: it is auto-generated

bb00c91

arielshaqed force-pushed the docs/6294-fix-broken-links branch from 38c25f3 to bb00c91 Compare August 3, 2023 07:25

arielshaqed mentioned this pull request Aug 3, 2023

Re-run Surge on pushes #6311

Closed

arielshaqed enabled auto-merge (squash) August 3, 2023 07:43

arielshaqed merged commit a77e9b7 into master Aug 3, 2023
33 checks passed

arielshaqed deleted the docs/6294-fix-broken-links branch August 3, 2023 07:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix broken links in documentation #6305

Fix broken links in documentation #6305

arielshaqed commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading

arielshaqed commented Aug 3, 2023

johnnyaug Aug 3, 2023

arielshaqed Aug 3, 2023

arielshaqed Aug 3, 2023

johnnyaug left a comment

arielshaqed left a comment

arielshaqed Aug 3, 2023

arielshaqed commented Aug 3, 2023

Fix broken links in documentation #6305

Fix broken links in documentation #6305

Conversation

arielshaqed commented Aug 2, 2023 • edited Loading

github-actions bot commented Aug 2, 2023 • edited Loading

arielshaqed commented Aug 3, 2023

Hiyush reviewers...

How to review

What to review

Why bother

johnnyaug Aug 3, 2023

Choose a reason for hiding this comment

arielshaqed Aug 3, 2023

Choose a reason for hiding this comment

arielshaqed Aug 3, 2023

Choose a reason for hiding this comment

johnnyaug left a comment

Choose a reason for hiding this comment

arielshaqed left a comment

Choose a reason for hiding this comment

arielshaqed Aug 3, 2023

Choose a reason for hiding this comment

arielshaqed commented Aug 3, 2023

arielshaqed commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading