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

docs-ci: Reduce unnecessary linkchecks #116

Open
victorlin opened this issue Nov 12, 2024 · 5 comments
Open

docs-ci: Reduce unnecessary linkchecks #116

victorlin opened this issue Nov 12, 2024 · 5 comments

Comments

@victorlin
Copy link
Member

Description

The purpose of running make linkcheck is to ensure:

  1. Newly added/modified links are valid
  2. Existing links are still valid

There was a huge benefit when adding this check, which surfaced all the invalid links that had been sitting unnoticed. However, I don't think we've gained much from it since then. The check is running on every single code change to a repo with a docs project. It's mostly been false-positives and I can only recall one true positive.

Possible solutions

  1. Run linkcheck on a schedule (e.g. weekly).
    • This removes the immediate feedback on newly added links, but I think it's acceptable for these reasons:
      1. It's unlikely that a PR author will add an invalid link.
      2. Most broken links do not need to immediately be fixed.
      3. For broken links that would benefit from immediate fix, a thorough review should catch these.
  2. Run linkcheck on CI but only when there are code changes in inputs.docs-directory.
    • This should be trivial to implement and maybe less controversial than (1).

priority: low To be resolved after high and moderate priority issues

With #106 resolved, things should be manageable. The benefits are:

  • Avoid causing confusion on some PRs. Errors from transient broken links are unnecessary noise on PRs that have not changed the link itself.
  • A reduction of unnecessary CPU cycles.
@genehack
Copy link
Contributor

This removes the immediate feedback on newly added links, but I think it's acceptable for these reasons:

The one issue I can see here is if somebody adds a new link, which works fine for interactive browsing, but not the automated link checking (a la most/all the 403s that we've ended up excluding). Of course, if people manually run the link check when adding new links, this won't be an issue, but that seems like exactly the sort of thing that will get skipped.

Can we combine 1 and 2? (I.e., run a weekly job that only triggers link checking if there was a change in inputs.docs-directory in the last week?)

@victorlin
Copy link
Member Author

Can we combine 1 and 2? (I.e., run a weekly job that only triggers link checking if there was a change in inputs.docs-directory in the last week?)

Best of both worlds! That sounds good to me.

@tsibley
Copy link
Member

tsibley commented Nov 12, 2024

Uh, that sounds like worst of both worlds? If the weekly job only runs when we change docs, then it won't catch when the external world changes and our unchanged links break. That's a huge point of linkcheck.

@victorlin
Copy link
Member Author

Oh right. Then (2) alone isn't great either. It would have to be (1) scheduled + (2) checks on PRs if we want immediate checks.

Right now we're relying on an abundance of (2) (without filtering to docs-only changes) to verify existing links.

@tsibley
Copy link
Member

tsibley commented Nov 12, 2024

Yep, indeed. I do the equivalent of (1) + (2) for the installation check I use for the CLI and it's been helpful.

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

No branches or pull requests

3 participants