TIDES-transit / TIDES

Transit ITS Data Exchange Specification for historical transit operations data
https://tides-transit.github.io/TIDES
Apache License 2.0
27 stars 4 forks source link

Fix Broken Docs Links #221

Open SorenSpicknall opened 3 months ago

SorenSpicknall commented 3 months ago

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.

github-actions[bot] commented 3 months ago

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

SorenSpicknall commented 3 months ago

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 commented 3 months ago

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:

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 commented 3 months ago

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.