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 link #1000

Merged
merged 2 commits into from
Sep 18, 2024
Merged

fix broken link #1000

merged 2 commits into from
Sep 18, 2024

Conversation

antonilol
Copy link
Contributor

again lol

@instagibbs
Copy link
Contributor

these are honestly all over the place in my experience, any way we can lint these?

@antonilol
Copy link
Contributor Author

i had some idea to list all links in the documents and all possible destinations and compare them (or something like this)

@antonilol antonilol marked this pull request as draft June 14, 2022 14:42
@antonilol
Copy link
Contributor Author

this is about what i imagined, it does not work yet and has bugs
it prints every link that points to something not found in any markdown file (*.md)

@vincenzopalazzo
Copy link
Contributor

I agree with the automatic check, but I disagree on the custom script, we should automate this process, and check with a Github action like this https://github.com/gaurav-nelson/github-action-markdown-link-check

We should ran this action periodically, maybe 1 time each month?

@antonilol
Copy link
Contributor Author

I agree with the automatic check, but I disagree on the custom script, we should automate this process, and check with a Github action like this https://github.com/gaurav-nelson/github-action-markdown-link-check

oh lol i did not search before making the script, but anyway it was fun to make

We should ran this action periodically, maybe 1 time each month?

i would suggest with every pull request

@vincenzopalazzo
Copy link
Contributor

i would suggest with every pull request

for sure, but also the url can be broken after X period of time, once the PR is landed (with valid URL) on the master

@antonilol antonilol marked this pull request as ready for review June 14, 2022 15:10
@antonilol
Copy link
Contributor Author

i would suggest with every pull request

for sure, but also the url can be broken after X period of time, once the PR is landed (with valid URL) on the master

ok yeah, a bit of an edge case, but good to also check those. i set it to every month and every pull request now

@antonilol
Copy link
Contributor Author

are actions disabled in this repo?

@instagibbs
Copy link
Contributor

wow, there's a script for this already? :D glad I asked

@antonilol
Copy link
Contributor Author

more people had issues with broken links before

@vincenzopalazzo
Copy link
Contributor

I think that the PR need to be merged before, otherwise creating a fake PR that trigger the CI can put down the Github action.

I had an attack like these in one of my repo, while a go now github disable it

@antonilol
Copy link
Contributor Author

antonilol commented Jun 14, 2022

ok i will merge it on my fork then to test it

testing it here: antonilol#1

@antonilol
Copy link
Contributor Author

it did not catch the error...

@antonilol antonilol force-pushed the broken-link-2 branch 2 times, most recently from e0f6d5b to f4647f6 Compare June 15, 2022 06:05
@t-bast
Copy link
Collaborator

t-bast commented Sep 18, 2024

it did not catch the error...

Does that mean the CI script you included doesn't work? Or am I misunderstanding? Not sure whether we should accept that change or just fix this specific broken link.

@antonilol
Copy link
Contributor Author

Does that mean the CI script you included doesn't work?

Yes. It reported no errors where there was at least one.
I will remove the commit that adds that CI script. We could try other link checkers and see if they do work. The link checker currently in this pr now recommends a different one in its readme.

@t-bast
Copy link
Collaborator

t-bast commented Sep 18, 2024

We could try other link checkers and see if they do work. The link checker currently in this pr now recommends a different one in its readme.

Let's try that in a separate PR to see if that works!

Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

Thanks!

@t-bast t-bast merged commit e6ee5f8 into lightning:master Sep 18, 2024
@antonilol antonilol deleted the broken-link-2 branch September 18, 2024 11:41
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.

4 participants