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

chore: add url_check.yml #231

Merged
merged 3 commits into from
Apr 4, 2024
Merged

Conversation

vil02
Copy link
Member

@vil02 vil02 commented Mar 24, 2024

Description

Related to #154. This PR adds a workflow checking all links in the markdown files.

There are too many broken links to fix them at once. I decided to change the workflow in such a way, that (temporary) it will not fail when broken links are found.

Checklist

  • A description of the changes in this PR is mentioned above.
  • All the new and existing checks pass.
  • No plagiarized, duplicated, or repetitive documentation that has been directly copied from another source.
  • If it's a new explanation, it contains solid, understandable, and accessible information.
  • I have read the whole contributing guidelines and agree to the Code of Conduct.

Screenshots (if any)

Note to reviewers

Adds a workflow checking links.

@vil02 vil02 force-pushed the add_url_check branch 5 times, most recently from b1f1581 to 3d25726 Compare March 24, 2024 20:21
@vil02 vil02 marked this pull request as ready for review March 24, 2024 20:34
Copy link
Collaborator

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

Useful as-is, since the CI output can be checked. Is there an (easy) option to make it fail if there are more broken links than there were before, though?

It would probably also be good to remind people in the contribution guidelines to use more permanent links (e.g. permalinks for Git repos, archive.org for unreliable or frequently changing websites etc.); we should also keep an eye on this when reviewing PRs: A link to a file in a Git repo is likely to break when the file is renamed. (And perhaps worse, the contents may change to not match the explanation, creating confusion).

@vil02
Copy link
Member Author

vil02 commented Mar 28, 2024

Is there an (easy) option to make it fail if there are more broken links than there were before, though?

I am not aware of an easy way. But I also think this is not the right way to go. One can split these broken links into three groups:

  1. the ones in the implementations-section: you suggested to remove these sections,
  2. links to images showing nicely the complexity, like O(n),
  3. all the others.

Two first groups are quite easy to fix (the second one just needs a guideline, so all of the formulas will be written in an uniform way). And I think there are not that many in the 3rd. I would suggest no broken links policy.

@appgurueu appgurueu mentioned this pull request Mar 29, 2024
@appgurueu
Copy link
Collaborator

appgurueu commented Mar 29, 2024

I am not aware of an easy way. But I also think this is not the right way to go.

Well, the idea was that if someone opens a PR, we would like for CI to check whether they are introducing any new broken links.

Two first groups are quite easy to fix (the second one just needs a guideline, so all of the formulas will be written in an uniform way). And I think there are not that many in the 3rd. I would suggest no broken links policy.

Yes, I agree. Though the policy should probably be "no broken links, and try to use permanent links (such as archive.org) that are unlikely to break". Most links were probably not broken when they were added, but broke later on.

I've opened a PR to update our policy.

@vil02
Copy link
Member Author

vil02 commented Apr 4, 2024

I would merge this as it is, remove all of the broken links (as mentioned above: some of them should be easy) and then make this workflow fail if any broken link is found. In the meantime one needs to manually inspect the log of this workflow.

@appgurueu appgurueu merged commit fe77013 into TheAlgorithms:master Apr 4, 2024
3 checks passed
@vil02 vil02 deleted the add_url_check branch April 4, 2024 17:16
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