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

Make markdown link checking only check internal links #4837

Closed
wants to merge 1 commit into from
Closed

Make markdown link checking only check internal links #4837

wants to merge 1 commit into from

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented May 24, 2022

Objective

  • This has begun spuriously failing again

Solution

  • Make it less likely to spuriously fail.
  • I've tried to test this locally, but the tool appears to just not do anything - I'm not sure what the output should be.

@james7132
Copy link
Member

Probably should just remove it altogether if it's still failing on this PR.

@cart
Copy link
Member

cart commented May 25, 2022

Yup I think we should remove it completely for now

@alice-i-cecile alice-i-cecile added the A-Build-System Related to build systems or continuous integration label May 25, 2022
@cart cart mentioned this pull request May 25, 2022
Closed
bors bot pushed a commit that referenced this pull request May 25, 2022
@cart
Remove markdown dead link check (#4839)
5dd30b6 
# Objective

This fails constantly and causes more pain than it is worth.

## Solution

Remove dead link checks.

Alternative to #4837, which is more granular but ironically still fails to build. I'm in favor of the nuclear option.

Fixes #4575
@james7132
Copy link
Member

Closing this as #4839 was merged.

@james7132 james7132 closed this May 25, 2022
@DJMcNab
Copy link
Member Author

DJMcNab commented May 25, 2022

I don't understand how this failure shows that we should remove it entirely? @james7132 can you explain the chain of logic there please?

Like obviously it's only failing here because I accidentally removed the config file reference whilst undoing #2551. I don't see how that failure has any bearing on whether it's useful to disallow invalid internal links, given we've had several instances in the past of such links and they're not easy to catch in review.

@james7132
Copy link
Member

Just didn't want to block other PRs until the config was fixed. If we still want those checks for the future, we can revert the removal with a more scoped config.

@DJMcNab DJMcNab deleted the bother-no-longer branch May 25, 2022 14:14
james7132 pushed a commit to james7132/bevy that referenced this pull request Jun 7, 2022
@cart @james7132
Remove markdown dead link check (bevyengine#4839)
5d49162 
# Objective

This fails constantly and causes more pain than it is worth.

## Solution

Remove dead link checks.

Alternative to bevyengine#4837, which is more granular but ironically still fails to build. I'm in favor of the nuclear option.

Fixes bevyengine#4575
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@cart @ItsDoot
Remove markdown dead link check (bevyengine#4839)
1224a07 
# Objective

This fails constantly and causes more pain than it is worth.

## Solution

Remove dead link checks.

Alternative to bevyengine#4837, which is more granular but ironically still fails to build. I'm in favor of the nuclear option.

Fixes bevyengine#4575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Build-System Related to build systems or continuous integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DJMcNab @james7132 @cart @alice-i-cecile