Skip to content

wip: TOC checking CI#4809

Draft
adamruzicka wants to merge 3 commits intotheforeman:masterfrom
adamruzicka:toc-ci
Draft

wip: TOC checking CI#4809
adamruzicka wants to merge 3 commits intotheforeman:masterfrom
adamruzicka:toc-ci

Conversation

@adamruzicka
Copy link
Copy Markdown
Contributor

@adamruzicka adamruzicka commented May 4, 2026

TODOs:

  • properly fill in the description template
  • rename the individual jobs to something more sensible

What changes are you introducing?

Adding a CI check that verifies that all the headings that the satellite branding plugin links to remain in place. It does so by:

  • building the satellite flavour of the docs
  • extracting TOC and aliases from the built html
  • runs the TOC checking rake from foreman_theme_satellite

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Every now and then seemingly innocent changes break the documentation link, leading to unnecessary back and forth. This change moves the check closer to the source.

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

For the time being, this is just a proof of concept. We may eventually find out it is too heavy handed.

Contributor checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.18/Katello 4.20 (Satellite 6.19)
  • Foreman 3.17/Katello 4.19
  • Foreman 3.16/Katello 4.18 (Satellite 6.18; orcharhino 7.6, 7.7, and 7.8)
  • Foreman 3.15/Katello 4.17
  • Foreman 3.14/Katello 4.16 (Satellite 6.17; orcharhino 7.4; orcharhino 7.5)
  • Foreman 3.13/Katello 4.15 (EL9 only)
  • Foreman 3.12/Katello 4.14 (Satellite 6.16; orcharhino 7.2 on EL9 only; orcharhino 7.3)
  • We do not accept PRs for Foreman older than 3.12.

This should go all the way back to 3.16 probably, but there will be some changes needed to get the branches right.

@github-actions github-actions Bot added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective labels May 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

The PR preview for cf3e753 is available at theforeman-foreman-documentation-preview-pr-4809.surge.sh

No diff compared to the current base

show diff

@adamruzicka adamruzicka force-pushed the toc-ci branch 2 times, most recently from 043a5e8 to 4854af5 Compare May 4, 2026 14:05
@Lennonka
Copy link
Copy Markdown
Contributor

Lennonka commented May 4, 2026

Huh, that's pretty good in principle. Thank you!

How difficult would it be to get an annotation on each module that would break a UI link, saying "You're about to break an UI link by changing this ID. Perhaps provide an alias in an auxiliary ID."?
It would be more user friendly for writers. A nice-to-have :)

Ps. I can help with job naming once we reach a conclusion.

@adamruzicka
Copy link
Copy Markdown
Contributor Author

How difficult would it be to get an annotation on each module that would break a UI link, saying "You're about to break an UI link by changing this ID. Perhaps provide an alias in an auxiliary ID."?
It would be more user friendly for writers. A nice-to-have :)

Because the link checking goes through two intermediate steps (adoc templates -> html -> toc), we lose the information about "this ID was in file F on line XY". However, since we have the list of "broken links" and the original diff, we should be able to piece it together in a best-effort fashion. It would still be a little bit tricky, but doable.

tl;dr: very difficult to get it 100% right every time, slightly involved to get it mostly right most of the times. Either way, I'd prefer to leave it as a followup

@Lennonka
Copy link
Copy Markdown
Contributor

Lennonka commented May 4, 2026

leave it as a followup

Totally cool with that.

@adamruzicka
Copy link
Copy Markdown
Contributor Author

Now I'd like to also turn this green. The tests are currently red due to recent rex guides restructuring in https://github.com/theforeman/foreman-documentation/pull/4753/files#issuecomment-4327312094 . Would just changing the leveloffsets do the trick or would that cause things to break somewhere else?

@Lennonka
Copy link
Copy Markdown
Contributor

Lennonka commented May 5, 2026

Do you mean changing leveloffsets in the toc generation? That might work, but I didn't check.
We can't change the leveloffsets in docs.

@Lennonka
Copy link
Copy Markdown
Contributor

Lennonka commented May 6, 2026

Looks like that fixed most problems, that's good!

However, the following issue will have to be fixed in the theme itself:

FAILED: Cannot find managing_hosts/index#Registering_Hosts_by_Using_Global_Registration_managing-hosts in TOC for entry: /Managing_Hosts/registering-a-host_managing-hosts

Global registration has been remade recently.

@adamruzicka
Copy link
Copy Markdown
Contributor Author

Going one level deeper in the toc seems to help, but I'd be slightly worried about (back)portability of this approach. The rex changes went all the way back to 3.16, in 3.16 the link checking mechanism works slightly differently so there might be some extra hurdles to overcome. This is a technical problem that can be resolved by a technical solution, but there is also a usability aspect to this.

If I recall correctly, the decision to limit the TOC to only upper three levels was to keep the surface area small (and ideally stable) and make sure that the in-project documentation links point to high level sections rather than pointing to overly low-level sections.

Also, PR#4793 brought us this this which will require changes in f-theme for develop and most likely auxiliary ids in stable branches of the docs. cc @aneta-petrova

FAILED: Cannot find managing_hosts/index#Registering_Hosts_by_Using_Global_Registration_managing-hosts in TOC for entry: /Managing_Hosts/registering-a-host_managing-hosts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs style review Requires a review from docs style/grammar perspective Needs tech review Requires a review from the technical perspective Not yet reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants