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 broken links #548

Merged
merged 18 commits into from
Mar 17, 2023
Merged

Fix broken links #548

merged 18 commits into from
Mar 17, 2023

Conversation

LorenzoMarnat
Copy link
Contributor

@LorenzoMarnat LorenzoMarnat commented Mar 3, 2023

The goal of this PR is to fix the links in UD-Viz documentation

remark -u validate-links . still returns 30 warnings (instead of 74)

@LorenzoMarnat LorenzoMarnat linked an issue Mar 3, 2023 that may be closed by this pull request
@valentinMachado valentinMachado self-assigned this Mar 3, 2023
@valentinMachado
Copy link
Contributor

Thanks for fixing these links !
But we should begin by this issue #516 because some markdown are going to move from ./packages/*/src/** to ./docs/static/** meaning the link are going to be broken again

@valentinMachado valentinMachado linked an issue Mar 6, 2023 that may be closed by this pull request
@valentinMachado
Copy link
Contributor

valentinMachado commented Mar 6, 2023

@LorenzoMarnat Iam going to solve #516 on your branch.

This is done with commit 2f2766f let's fix these links !

@valentinMachado
Copy link
Contributor

I put the FAILURE_THRESHOLD at 0 to detect every dead link

@DiegoVinasco
Copy link
Contributor

This is great. In addition to this work however, we might want to consider a flaw with remark validate links...

Bascially, Remark doesn't validate absolute links (which are accepted by github in markdown documents). It only checks relative links or URLs.

This means that we might still have some broken links. This was worked around in the private repository here by using a sed command with some pipes to prefix absolute markdown links with a given URL:

export BASE_URI='https://github.com/VCityTeam/VCity/blob/master'
files=$(find . -name '*.md')
for f in $files
do
  grep -E '\[{1}.*\]{1}\({1}/{1}.*\){1}' "$f" | sed -i "s|](/|]($BASE_URI/|g" "$f"
done
remark -u validate-links .

I think it would be a good idea to do the same in UD-Viz. Maybe in a different PR? Any thoughts?

@valentinMachado
Copy link
Contributor

This is great. In addition to this work however, we might want to consider a flaw with remark validate links...

Bascially, Remark doesn't validate absolute links (which are accepted by github in markdown documents). It only checks relative links or URLs.

This means that we might still have some broken links. This was worked around in the private repository here by using a sed command with some pipes to prefix absolute markdown links with a given URL:

export BASE_URI='https://github.com/VCityTeam/VCity/blob/master'
files=$(find . -name '*.md')
for f in $files
do
  grep -E '\[{1}.*\]{1}\({1}/{1}.*\){1}' "$f" | sed -i "s|](/|]($BASE_URI/|g" "$f"
done
remark -u validate-links .

I think it would be a good idea to do the same in UD-Viz. Maybe in a different PR? Any thoughts?

Since this PR is called "Fix broken links" imo we should let this open. I'm not sure to understand why there could be absolute link in .md, could you give an example of link that remark doesnt detect ?

@valentinMachado
Copy link
Contributor

valentinMachado commented Mar 9, 2023

Actually absolute link should be consider as a bad practice since the root of the path will depend of the context in which it's interpreted (like vscode workspaces or github organization).

Iam going to add in the script validate-links a code to detect absolute links and throw an error if one is found.

Done, waiting for a review

@valentinMachado valentinMachado linked an issue Mar 9, 2023 that may be closed by this pull request
docs/static/Devel/Developers.md Show resolved Hide resolved
docs/static/Devel/LocalGameTutorial.md Show resolved Hide resolved
packages/node/Readme.md Show resolved Hide resolved
packages/browser/Readme.md Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@valentinMachado valentinMachado merged commit 95fead7 into master Mar 17, 2023
@valentinMachado valentinMachado deleted the fix_links branch March 17, 2023 15:43
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.

detect absolute links reduce npm run validate-links threshold to 9 Doc : static doc file in src
5 participants