-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-141004: GHA: Run check-c-api-docs check on docs-only PRs
#143573
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
Conversation
@ZeroIntensity
ZeroIntensity
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me, but I'd like one of the GHA experts to have a look.
encukou
commented
Jan 9, 2026
@webknjaz, does this look interesting?
Personally, I've been leaning towards having a single workflow tool target invocation per job. In this case it's one make command (I've implemented my reusable-tox.yml thing around the same approach and a it turned out to be a pretty powerful strategy).
So yes, it's a good idea — it can run a check that would otherwise be skipped, sometimes. But I also foresee better responsiveness even in full runs. This could be taken farther at some point (follow-ups) but it's well-scoped as it is.
One thing I'd ask, though would be moving the new job into a reusable-*.yml module so this doesn't contribute to cluttering the huge YAML file more than it is already. No other comments otherwise.
On this PR, the new job ran in 13 seconds.
In that case, I'd maybe reduce the job timeout to 5 minutes or less. This tends to improve responsiveness too, plus catch problems with sudden slowdowns w/o wasting too much CPU.
StanFromIreland
commented
Jan 9, 2026
One thing I'd ask, though would be moving the new job into a reusable-*.yml module
...
reduce the job timeout to 5 minutes
Done :-)
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@webknjaz
webknjaz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready, I left a style note, though. But that's inconsistent across the CI definitions anyway, so not important.
encukou
commented
Jan 12, 2026
As I replied to the comment mentioned in the OP:
If we do [this], we should take the names from the newly-built Intersphinx inventory, rather than parse the .rst.
Here's the code to parse Intersphinx inventory: main...encukou:cpython:doccheck-intersphinx
Would it be possible to hook up GHA so that the Doc/build/html/objects.inv file generated in the "Doc build" step is accessible? And perhaps use a slightly older one if docs weren't rebuilt?
(Not necessarily in this PR of course.)
Would it be possible to hook up GHA so that the Doc/build/html/objects.inv file generated in the "Doc build" step is accessible?
Yes we could, we can upload the artifact in the docs build and run the c-api check when it is available.
Edit: Funnily enough, here is an example of just that: #143742
Since it is not necessary yet, I suggest it is done in a future PR.
StanFromIreland
commented
Jan 12, 2026
I can open a PR on your fork against your branch with the changes when this is merged.
encukou
commented
Jan 13, 2026
Thank you!
Feel free to send PRs to my branch. Or merge to your branch and send the CPython PR from that.
webknjaz
commented
Jan 13, 2026
Here's the code to parse Intersphinx inventory: main...encukou:cpython:doccheck-intersphinx
Or from sphinx.ext.intersphinx import fetch_inventory if Sphinx is available in that env. I'm using it in a hacky static website that shows tables with linckable objects on the web: https://webknjaz.github.io/intersphinx-untangled/.
encukou
commented
Jan 13, 2026
fetch_inventory is undocumented; it needs a Sphinx app and returns an Inventory which are both also undocumented. I don't think it's much of a win over zlib...
(Are you sure your site works with the latest Sphinx?)
Are you sure your site works with the latest Sphinx?
Yeah, I was lazy and didn't pin versions. So it always pulls whatever's latest on PyPI (every (削除) three (削除ここまで) four hours): https://github.com/webknjaz/intersphinx-untangled/blob/dbb400751a169988d486495b160137fc2a594438/.github/workflows/build-gh-pages.yml#L26C48-L26C54. On that note, I just realized I'm passing a fake object in there (see a dozen lines below). YOLO!
Uh oh!
There was an error while loading. Please reload this page.
As mentioned in #143564 (review) by @ZeroIntensity:
We could keep it in the
check-generated-filesjob, but that would make it quite messy as all the other checks would have to be excluded in the case when onlyrun-docsis true. And, to avoid having to configure, I run the script directly (since it usesPYTHON_FOR_REGENanyway, there shouldn't be a difference).On this PR, the new job ran in 13 seconds.