Skip to content

gh-145000: Run check-html-ids.py in CI#145632

Merged
StanFromIreland merged 15 commits intopython:mainfrom
StanFromIreland:check-html-ids.py-ci
Apr 1, 2026
Merged

gh-145000: Run check-html-ids.py in CI#145632
StanFromIreland merged 15 commits intopython:mainfrom
StanFromIreland:check-html-ids.py-ci

Conversation

@StanFromIreland
Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland commented Mar 7, 2026

This doesn’t introduce any slowdown in the CI as the doctest job is slower. To avoid building the docs twice, I use the build from the build-doc step to run the script and upload the results.

🟢 Pass example
🔴 Fail example (When merged, it will properly fail, currently the script doesn’t sys.exit(1))


📚 Documentation preview 📚: https://cpython-previews--145632.org.readthedocs.build/

@StanFromIreland
Copy link
Copy Markdown
Member Author

StanFromIreland commented Mar 7, 2026

Annoyingly, no exclude file exists on main yet, and trying to add one here won't work, since the PR base won't have it. So, I am forced to leave a TODO.

And we have the same issue with adding sys.exit(1) to fail the job, it's not on main yet.

@StanFromIreland StanFromIreland marked this pull request as ready for review March 7, 2026 20:52
@StanFromIreland StanFromIreland requested a review from encukou March 7, 2026 20:52
@webknjaz
Copy link
Copy Markdown
Member

webknjaz commented Mar 8, 2026

I don't like that reusable-*.yml modules are getting more and more jobs. I was hoping to keep them with one job each with any info exchange through inputs/outputs...

@StanFromIreland
Copy link
Copy Markdown
Member Author

I don't like that reusable-*.yml modules are getting more and more jobs. I was hoping to keep them with one job each with any info exchange through inputs/outputs...

In this instance, I think it is better to group them, creating a new module will just introduce more issues with passing inputs IMO.

@hugovk
Copy link
Copy Markdown
Member

hugovk commented Mar 9, 2026

So we're building docs in the PR, then checking out the base branch, building the docs again, and comparing?

It only takes a couple of minutes for that base branch build, but it feels like we could avoid this extra CI work somehow?

Of course, we don't build docs for every PR, so that reduces it. And we also don't build docs for regular pushes to main, which complicates things.

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@StanFromIreland StanFromIreland requested a review from hugovk March 9, 2026 22:28
@encukou
Copy link
Copy Markdown
Member

encukou commented Mar 10, 2026

So we're building docs in the PR, then checking out the base branch, building the docs again, and comparing?

Yeah, that's wasteful.
I was thinking about building and publishing these on docs.python.org. Would that make sense to you?

@StanFromIreland
Copy link
Copy Markdown
Member Author

I was thinking about building and publishing these on docs.python.org. Would that make sense to you?

Then we'd have to host it for every commit, or accept that much more frequent branch updates are necessary (which is also wasteful). For example, commits:

main
- A
- B (Adds an ID)

PR:
- A (base)
# Behind, missing B
- C (Unrelated changes by the PR)

When the CI runs on C, it will pull the base IDs. But, the IDs added by B will be missing at C, so the CI will falsely report it as removed.

@StanFromIreland
Copy link
Copy Markdown
Member Author

For the record, we discussed this during Petr's docs meeting yesterday.

  • We concluded that storing and pulling from docs.python.org won't work, for the reason outline in gh-145000: Run check-html-ids.py in CI #145632 (comment).
  • We then considered merging in the main branch, then building and pulling the base from from docs.python.org. However, we found this would be quite complicated, and would also cause issues in some cases, and as such is not worth it.
  • We decided we don't need to run the check on push. (Sending a commit to address this)

@StanFromIreland
Copy link
Copy Markdown
Member Author

I don't like that reusable-*.yml modules are getting more and more jobs. I was hoping to keep them with one job each with any info exchange through inputs/outputs...

After some more thought, it does not actually complicate anything, and as such, I have decided to do this.

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@StanFromIreland StanFromIreland requested a review from hugovk March 31, 2026 13:51
@StanFromIreland StanFromIreland requested a review from hugovk March 31, 2026 14:58
@StanFromIreland StanFromIreland merged commit 08c5d3d into python:main Apr 1, 2026
94 of 95 checks passed
@StanFromIreland StanFromIreland deleted the check-html-ids.py-ci branch April 1, 2026 13:10
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 1, 2026

This change introduced errors in the GHA Lint CI: https://github.com/python/cpython/actions/runs/23850561184/job/69529219964?pr=147962 (from PR gh-147962)

Example:

  error[: unpinned action reference
    --> .github/workflows/reusable-check-html-ids.yml:19:15
     |
  19 |         uses: actions/checkout@v6
     |               ^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
     |
     = note: audit confidence → High
     = help: audit documentation → https://docs.zizmor.sh/audits/#unpinned-uses

Since this PR was created (1 month ago), zizmor was made more strict (see commit a504c0a of issue gh-146488).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants