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.

Sign up for GitHub

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

Jump to bottom

Multiple broken links in documentation #1092

Closed
krystian-hebel opened this issue Oct 16, 2024 · 8 comments
Closed

Multiple broken links in documentation #1092

krystian-hebel opened this issue Oct 16, 2024 · 8 comments
Assignees
@filipleple
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request needs review

Comments

@krystian-hebel
Copy link

Component

Dasharo documentation

Device

Protectli FW6, Protectli VP2410, Protectli VP2420, Protectli VP4630, Protectli VP4650, Protectli VP4670, Protectli VP6630, Protectli VP6650, Protectli VP6670, other

Dasharo version

No response

Dasharo Tools Suite version

No response

Test case ID

No response

Brief summary

Review done before merging PRs to documentation is inadequate. Output of mkdocs serve shows a warning for each broken link, but it is hidden between a few hundred INFO lines.

How reproducible

No response

How to reproduce

Click on Releases on https://docs.dasharo.com/unified/protectli/overview/#fw6

Expected behavior

Browser navigates to list of releases.

Actual behavior

404

Screenshots

No response

Additional context

https://paste.dasharo.com/?c7e5b8307c34a8ed#7HZvr9UpGctQxiXiSCe35cQNXJhKrdGJfgoRgzrvYojc - this is an output of mkdocs serve on current master. It has 300 informative lines about absolute links (which most likely will break when the files are moved around, e.g. to unified directory) and 22 warnings about broken links. Not all of the warnings are printed at the end, so one has to scroll the screen through all the INFO noise to notice them.

Solutions you've tried

Similar issue happened when I was working on TrenchBoot documentation, the easy-but-tedious solution is to do what mkdocs suggests and change all absolute links to relative ones. This will get rid of almost all of the INFOs (only last few lines are left, with time summary and link to local preview), which makes it easier to focus on real issues.

Another part of the problem is that we currently don't demand from neither authors nor reviewers to check the output of mkdocs serve. This can be fixed by adding instruction to README, e.g. like this one.
@krystian-hebel krystian-hebel added bug Something isn't working needs review labels Oct 16, 2024
@krystian-hebel krystian-hebel added documentation Improvements or additions to documentation enhancement New feature or request and removed protectli_vault_kbl FW6A, FW6B, FW6C, FW6D, FW6E protectli_vault_glk VP 2410 protectli_vault_cml VP 4xxx protectli_vault_ehl eg. VP2420 protectli_vault_adl doc labels Oct 16, 2024
@macpijan
Copy link
Contributor

Another part of the problem is that we currently don't demand from neither authors nor reviewers to check the output of mkdocs serve. This can be fixed by adding instruction to README, e.g. like this one.

Sound like could be added to CI checks as well?

@krystian-hebel
Copy link
Author

Sound like could be added to CI checks as well?

Yes, if possible.

@macpijan
Copy link
Contributor

OTOH

image

Do you think we need the links on the right in body, if we have the same links in navigation tab on the left?

@krystian-hebel
Copy link
Author

Probably not, but it would be empty without those, and other platforms have more content in these tabs.

@macpijan
Copy link
Contributor

@BeataZdunczyk We have to handle that, please review these pages, I am in favor of removing the duplicate links since we already have them in the navigation tab. Especially if it would simplify the work to be done with relative links as suggested by Krystian.

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 17, 2024 

�[33mWARNING -  �[0mRedirect target 'unified/protectli/releases.md#fw6' does not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/releases.md#v1000-series' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/releases.md#vp46xx' does not
           exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/releases.md#vp66xx' does not
           exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/releases.md#vp2410' does not
           exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/releases.md#vp2420' does not
           exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/building_manual.md#fw6' does not
           exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/building_manual.md#v1000-series'
           does not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/building_manual.md#vp46xx' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/building_manual.md#vp66xx' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/building_manual.md#vp2410' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/building_manual.md#vp2420' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/firmware_update.md#fw6' does not
           exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/firmware_update.md#v1000-series'
           does not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/firmware_update.md#vp46xx' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/firmware_update.md#vp66xx' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/firmware_update.md#vp2410' does
           not exist!
�[33mWARNING -  �[0mRedirect target 'unified/protectli/firmware_update.md#vp2420' does
           not exist!

Not sure about this one but rediretcs are working. Maybe the syntax should be different...

I have already fixed a couple of rebase/squash mistakes for the PR 851 already:
Dasharo/docs#902
Dasharo/docs@7be93ba (when noticed old links give 404).

@filipleple
Copy link
Member

So far I've managed to get it down to

INFO    -  Building documentation...
INFO    -  Cleaning site directory
INFO    -  The following pages exist in the docs directory, but are not included in the "nav" configuration:
  - variants/hardware-compatibility-list.md
  - variants/msi_z690/building-manual.md
  - variants/msi_z690/cpu-hcl.md
  - variants/msi_z690/development.md
  - variants/msi_z690/faq.md
  - variants/msi_z690/firmware-update.md
  - variants/msi_z690/gpu-hcl.md
  - variants/msi_z690/initial-deployment.md
  - variants/msi_z690/memory-hcl.md
  - variants/msi_z690/openness_analysis.md
  - variants/msi_z690/overview.md
  - variants/msi_z690/recovery.md
INFO    -  Doc file 'unified-test-documentation/dasharo-compatibility/320-fwupd-firmware-update.md' contains an absolute link '/unified/novacustom/fwupd-usage', it was left as is.
INFO    -  Doc file 'unified-test-documentation/dasharo-compatibility/320-fwupd-firmware-update.md' contains an absolute link '/unified/novacustom/fwupd-usage', it was left as is.
INFO    -  Documentation built in 5.65 seconds
INFO    -  [19:16:30] Watching paths for changes: 'docs', 'mkdocs.yml'
INFO    -  [19:16:30] Serving on http://127.0.0.1:8000/
INFO    -  [19:16:34] Browser connected: http://127.0.0.1:8000/unified/protectli/overview/
INFO    -  Shutting down...

@filipleple
Copy link
Member

All problems gone as of Dasharo/docs#924

@macpijan macpijan closed this as completed Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@filipleple filipleple

Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request needs review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@macpijan @miczyg1 @krystian-hebel @filipleple