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 Docs Links #221

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Fix Broken Docs Links #221

wants to merge 11 commits into from

Conversation

SorenSpicknall
Copy link
Contributor

@SorenSpicknall SorenSpicknall commented Jul 29, 2024

Resolves #209 by updating the dictionary of link substitutions referenced during docs generation (which is used in our setup because we include_file in various places where the included file sits at a different folder level than the file it's included within). Also makes a couple small copy edits, where found.

I did a quick click-through of links within the docs site on this branch to see if there were any additional broken links beyond the ones already identified on the Development page, and did not find any.

Copy link
Contributor

Documentation available at: http://tides-transit.github.io/TIDES/soren-docs_linkfixes

@SorenSpicknall
Copy link
Contributor Author

SorenSpicknall commented Jul 29, 2024

Leaving this for now so that I can think on it - an issue is occurring in the docs build process, related to the release of setuptools 72.0.0 at just the right time to mess with this testing (approximately 20 minutes before I opened this PR). Because of that, I've so far been unable to test what I meant to test with this PR.

72.0.0 removed the test module from setuptools, a module which appears to be referenced somewhere in our dependencies or in our baseline Python build setup for CI. I've been unable to track its exact source so far - when I explicitly swap an earlier version of the package into requirements.txt, preempting the install via mkdocs-mermaid2-plugin, the logs I get back still seem to reference errors occurring on lines that map to the latest version of setuptools, rather than the earlier version I specified. Such as here, where line 313 is referenced in logs for error handling that takes place on line 311 in the specified version of setuptools (70.0.0).

I'm coming back to this later in part because there could be some weird caching behavior going on here with dependencies in the GitHub Action, in part because time away might give me the energy to attempt to detect where the wrong setuptools version is coming from, and in part because if this is due to a functionality/distribution bug for setuptools 72.0.0, it may be resolved by that contributor community if I give it a day or two.

As suspected, last night's build woes had to do with the release of setuptools 72.0.0, which was yanked this morning and replaced with 72.1.0 shortly thereafter. Some TIDES dep (seemingly a CI dep, which we don't have direct control over) was referencing a deprecated module that got removed in 72.0.0, breaking CI. Now the build process is working correctly, which will allow me to proceed with my intended work on this PR.

@SorenSpicknall SorenSpicknall marked this pull request as ready for review July 31, 2024 17:35
@SorenSpicknall SorenSpicknall requested a review from a team as a code owner July 31, 2024 17:35
@SorenSpicknall SorenSpicknall changed the title WIP: Fix Broken Docs Links Fix Broken Docs Links Jul 31, 2024
Copy link
Contributor

@botanize botanize left a comment

Choose a reason for hiding this comment

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

This fixes the one identified broken link. Not sure what the others are that are alluded to in the issue. Given that this link re-writing is fairly fragile, would it be possible to add CI to check links?

@SorenSpicknall
Copy link
Contributor Author

SorenSpicknall commented Jul 31, 2024

This fixes the one identified broken link. Not sure what the others are that are alluded to in the issue.

There were a few additional broken links on the same page - the following link definitions were all functioning in the original CONTRIBUTING.md but nonfunctioning in the built + published Development page because that page utilized include_file without defining remappings for all the relevant links that came from CONTRIBUTING.md:

  • doc-building
  • CLA
  • change-policy
  • code of conduct

I checked each of the other docs pages and didn't find instances of the same issue.

Given that this link re-writing is fairly fragile, would it be possible to add CI to check links?

I agree that we should add a CI check that crawls the docs site and checks for 404s among links. My spare time to contribute to TIDES is pretty limited at the moment and I can't tackle that immediately, so is it alright with you if I document that idea via a new Issue and merge this set of changes, @botanize?

EDIT: realized the main branch is protected right now. These changes are non-normative according to the Change Management Policy, I believe, but I'm unsure what I'm supposed to do post-approval here. The develop branch doesn't currently exist to target, but I could create it from main and retarget the PR if that's the right move.

@botanize
Copy link
Contributor

botanize commented Aug 1, 2024

Given that this link re-writing is fairly fragile, would it be possible to add CI to check links?

I agree that we should add a CI check that crawls the docs site and checks for 404s among links. My spare time to contribute to TIDES is pretty limited at the moment and I can't tackle that immediately, so is it alright with you if I document that idea via a new Issue and merge this set of changes, @botanize?

💯

EDIT: realized the main branch is protected right now. These changes are non-normative according to the Change Management Policy, I believe, but I'm unsure what I'm supposed to do post-approval here. The develop branch doesn't currently exist to target, but I could create it from main and retarget the PR if that's the right move.

I don't know the process anymore either, probably easiest to check with @jlstpaul.

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.

🐛 Broken Links
2 participants