X Tutup
Skip to content

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

Open
StanFromIreland wants to merge 8 commits intopython:mainfrom
StanFromIreland:check-html-ids.py-ci
Open

gh-145000: Run check-html-ids.py in CI#145632
StanFromIreland wants to merge 8 commits intopython:mainfrom
StanFromIreland:check-html-ids.py-ci

Conversation

@StanFromIreland
Copy link
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
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
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
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
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.

uses: actions/upload-artifact@v6
with:
name: html-ids-head
path: Doc/build/html-ids-head.json.gz
Copy link
Member

Choose a reason for hiding this comment

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

This is already compressed, we can skip zipping it:

https://github.blog/changelog/2026-02-26-github-actions-now-supports-uploading-and-downloading-non-zipped-artifacts/

Suggested change
path: Doc/build/html-ids-head.json.gz
path: Doc/build/html-ids-head.json.gz
archive: false

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, this is causing issues, I will look into it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hugo, FYI, this requires updtaing the name.

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
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
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.

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.

4 participants

X Tutup