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

Fixed #2364 by adding a Github workflow for checking dead links #2406

Merged
merged 9 commits into from
Jul 6, 2023
Merged

Fixed #2364 by adding a Github workflow for checking dead links #2406

merged 9 commits into from
Jul 6, 2023

Conversation

SahibYar
Copy link
Contributor

@SahibYar SahibYar commented Jul 2, 2023

.github/actions/go-test-setup/action.yml Outdated Show resolved Hide resolved
.github/workflows/link-check.yml Outdated Show resolved Hide resolved
.github/workflows/link-check.yml Outdated Show resolved Hide resolved
.github/workflows/markdown-links-config.json Outdated Show resolved Hide resolved
"pattern": "^http://localhost"
},
{
"pattern": "^https://twitter.com/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

@SahibYar SahibYar Jul 3, 2023

Choose a reason for hiding this comment

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

if we don't add this, our workflow will always fail because

curl --verbose --location 'https://twitter.com/awalterschulze/status/1584553056100057088'

My Github action was showing error like this

FILE: ./CHANGELOG.md
[✖] https://github.com/libp2p/go-libp2p/blob/master/p2p/net/swarm/grafana-dashboards/swarm.json → Status: 404
[✖] https://github.com/libp2p/go-libp2p/blob/master/p2p/host/eventbus/grafana-dashboards/eventbus.json → Status: 404
[✖] https://twitter.com/awalterschulze/status/1584553056100057088 → Status: 0 Error: Exceeded maxRedirects. Probably stuck in a redirect loop https://twitter.com/awalterschulze/status/1584553056100057088
    at Redirect.onResponse (/usr/local/lib/node_modules/markdown-link-check/node_modules/request/lib/redirect.js:98:27)
    at Request.onRequestResponse (/usr/local/lib/node_modules/markdown-link-check/node_modules/request/request.js:986:22)
    at ClientRequest.emit (node:events:513:28)
    at HTTPParser.parserOnIncomingClient (node:_http_client:701:27)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17)
    at TLSSocket.socketOnData (node:_http_client:542:22)
    at TLSSocket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)
    at Readable.push (node:internal/streams/readable:234:10)

132 links checked.

ERROR: 3 dead links found!
[✖] https://github.com/libp2p/go-libp2p/blob/master/p2p/net/swarm/grafana-dashboards/swarm.json → Status: 404
[✖] https://github.com/libp2p/go-libp2p/blob/master/p2p/host/eventbus/grafana-dashboards/eventbus.json → Status: 404
[✖] https://twitter.com/awalterschulze/status/[158](https://github.com/SahibYar/go-libp2p/actions/runs/5433440994/jobs/9881111425#step:4:159)4553056100057088 → Status: 0

Details: gaurav-nelson/github-action-markdown-link-check#182 (comment)

Copy link
Collaborator

@MarcoPolo MarcoPolo Jul 6, 2023

Choose a reason for hiding this comment

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

Twitter links are auth-walled so there's no way to check if the link works

.github/workflows/markdown-links-config.json Outdated Show resolved Hide resolved
.github/workflows/markdown-links-config.json Outdated Show resolved Hide resolved
@p-shahi p-shahi linked an issue Jul 6, 2023 that may be closed by this pull request
@SahibYar
Copy link
Contributor Author

SahibYar commented Jul 6, 2023

Thank you @MarcoPolo for your review, I have updated PR as per your suggestion.

push:
branches:
- "master"
schedule:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would we want this on a schedule? Seems fine to do it on PRs. That will be more often than at 9am on the second day of the month.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this based on a comment from @marten-seemann on ticket.
#2364 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is there a need to revert this change ?
5ea669d

@MarcoPolo
Copy link
Collaborator

Looks like this found some dead links! @SahibYar can you please fix those links?

@SahibYar
Copy link
Contributor Author

SahibYar commented Jul 6, 2023

Looks like this found some dead links! @SahibYar can you please fix those links?

I tried fixing some links which @marten-seemann said these unrelated to this PR.
0b77b69#r1249734957

@MarcoPolo
Copy link
Collaborator

Looks like this found some dead links! @SahibYar can you please fix those links?

I tried fixing some links which @marten-seemann said these unrelated to this PR. 0b77b69#r1249734957

I can understand the desire to keep the diff small. But we can’t merge if ci is failing. Can you fix the links in a single separate commit?

@SahibYar
Copy link
Contributor Author

SahibYar commented Jul 6, 2023

Looks like this found some dead links! @SahibYar can you please fix those links?

I tried fixing some links which @marten-seemann said these unrelated to this PR. 0b77b69#r1249734957

I can understand the desire to keep the diff small. But we can’t merge if ci is failing. Can you fix the links in a single separate commit?

sure

@SahibYar
Copy link
Contributor Author

SahibYar commented Jul 6, 2023

Looks like this found some dead links! @SahibYar can you please fix those links?

I tried fixing some links which @marten-seemann said these unrelated to this PR. 0b77b69#r1249734957

I can understand the desire to keep the diff small. But we can’t merge if ci is failing. Can you fix the links in a single separate commit?

PR updated .

@MarcoPolo
Copy link
Collaborator

Thanks @SahibYar!

@MarcoPolo MarcoPolo merged commit fa153c5 into libp2p:master Jul 6, 2023
@MarcoPolo MarcoPolo mentioned this pull request Jul 14, 2023
29 tasks
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.

ci: add a link checker GitHub Action cron job
3 participants