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 links on main doc page #1717

Merged
merged 4 commits into from
Dec 9, 2022
Merged

Fix links on main doc page #1717

merged 4 commits into from
Dec 9, 2022

Conversation

FWDekker
Copy link
Contributor

@FWDekker FWDekker commented Nov 30, 2022

Description

The website's front page has a few broken links. They use anchors (e.g. #editorconfig), but clicking them doesn't do anything.

I tried to fix them by changing them to absolute links. I considered using relative links, but saw that that page also has other absolute links to the documentation itself.

I wasn't sure where to link the #editorconfig to, because there are multiple places where EditorConfig is discussed in different ways. In the end I just guessed which one would be more appropriate, but maybe the right answer was to remove the link?

Checklist

PS: I just noticed there are also other dead links on the website :/
PPS: In the rule configuration section, there is a link to another rule configuration section, but that page is totally broken. Would you like me to open an issue for that?

@paul-dingemans
Copy link
Collaborator

Tnx for taking the effort to improve the documentation. Maintaining the links is indeed painful and error prone. I thought that I checked them all when I rewrote the documentation a while ago. I am glad that you found them and reported them. Ideally, I prefer relative links as those can be tested on the local machine (see https://github.com/pinterest/ktlint/blob/master/docs/readme.md).

The broken page should refer to https://pinterest.github.io/ktlint/rules/configuration-ktlint/.

I would really appreciate if you could contribute by reviewing and fixing the ktlint documentation.

@FWDekker
Copy link
Contributor Author

FWDekker commented Dec 2, 2022

I've used relative links and fixed some links on other pages as well (on my local branch, not pushed yet), but I see now that the Extensions section often refers to https://pinterest.github.io/ktlint-core/ for the source code. This website redirects to https://www.pinterestcareers.com/ instead of showing source code. Should I change these links to this GitHub repo?

@paul-dingemans
Copy link
Collaborator

I've used relative links and fixed some links on other pages as well (on my local branch, not pushed yet), but I see now that the Extensions section often refers to https://pinterest.github.io/ktlint-core/ for the source code. This website redirects to https://www.pinterestcareers.com/ instead of showing source code. Should I change these links to this GitHub repo?

Yes please do change them as well. They are obviously wrong.

@paul-dingemans
Copy link
Collaborator

paul-dingemans commented Dec 4, 2022

I have just used the Broken Link Checker Tool on the https://pinterest.github.io/ktlint/. 25 broken links were found. Would you mind and try to fix them? It would also be valuable to document this approach in https://github.com/pinterest/ktlint/blob/master/RELEASE_TESTING.MD so that it at least get checked before a release.

For now, you can ignore links to https://ktlint.github.io/. I expect that those links are caused by this line https://github.com/pinterest/ktlint/blob/master/mkdocs.yml#L2. Once that line is updated, I hope that the broken links referring to that address will be gone.

@FWDekker FWDekker closed this Dec 6, 2022
@FWDekker FWDekker deleted the patch-1 branch December 6, 2022 17:22
@FWDekker FWDekker reopened this Dec 6, 2022
@FWDekker
Copy link
Contributor Author

FWDekker commented Dec 7, 2022

I accidentally deleted my branch at some point somehow, but I think I've finished the PR now! ^^

All dead links have been resolved, all links have been made relative, I have documented the checks to do before release, and I expanded docs/readme.md to describe how to locally build the site (so that I could upload it to my own website and then use the Broken Link Checker Tool there to verify that all links were fixed (except those resulting from the base-url being incorrect, but that will be fixed with #1721)). Because the latter procedure builds the website in site/, I added this directory to the .gitignore to make sure nobody accidentally commits the built website (which I almost accidentally did at some point).

What do you think? Let me know if you spot anything that's still missing or could be better!

@FWDekker
Copy link
Contributor Author

FWDekker commented Dec 7, 2022

I forgot to mention, there is still one dead link that I cannot solve on my own. On installation/overview, there is a link to https://ktlint-demo.herokuapp.com/ to try out KtLint online. However, that website doesn't work.

@paul-dingemans
Copy link
Collaborator

I accidentally deleted my branch at some point somehow, but I think I've finished the PR now! ^^

All dead links have been resolved, all links have been made relative, I have documented the checks to do before release, and I expanded docs/readme.md to describe how to locally build the site (so that I could upload it to my own website and then use the Broken Link Checker Tool there to verify that all links were fixed (except those resulting from the base-url being incorrect, but that will be fixed with #1721)). Because the latter procedure builds the website in site/, I added this directory to the .gitignore to make sure nobody accidentally commits the built website (which I almost accidentally did at some point).

What do you think? Let me know if you spot anything that's still missing or could be better!

This is looking awesome. I am very glad and pleased that you improved the documentation.

@paul-dingemans
Copy link
Collaborator

I forgot to mention, there is still one dead link that I cannot solve on my own. On installation/overview, there is a link to https://ktlint-demo.herokuapp.com/ to try out KtLint online. However, that website doesn't work.

This a known problem. See saveourtool/diktat-demo#66

@paul-dingemans paul-dingemans merged commit 980a7ac into pinterest:master Dec 9, 2022
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.

2 participants