GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-141004: GHA: Run `check-c-api-docs` check on docs-only PRs by StanFromIreland · Pull Request #143573 · python/cpython

Conversation

@StanFromIreland

@StanFromIreland StanFromIreland commented Jan 8, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

As mentioned in #143564 (review) by @ZeroIntensity:

One extra thing I would like to do is run the script on docs PRs, since a name being documented while also being in ignored_c_api.txt will cause it to fail. So, if someone merges new documentation without updating ignored_c_api.txt, we'll see blocking failures on all C code PRs until we fix it. I haven't looked too closely at the GHA for change detection, but if you see an easy way to do that, would you mind doing it in this PR?

We could keep it in the check-generated-files job, but that would make it quite messy as all the other checks would have to be excluded in the case when only run-docs is true. And, to avoid having to configure, I run the script directly (since it uses PYTHON_FOR_REGEN anyway, there shouldn't be a difference).

On this PR, the new job ran in 13 seconds.

  • Issue: Document the entire C API #141004

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@StanFromIreland
Run on docs
e4fae10
@StanFromIreland
StanFromIreland requested a review from ZeroIntensity January 8, 2026 16:45
@StanFromIreland
StanFromIreland requested review from AA-Turner, ezio-melotti and hugovk as code owners January 8, 2026 16:45
@StanFromIreland StanFromIreland added the skip news label Jan 8, 2026
@bedevere-app bedevere-app Bot added the awaiting review label Jan 8, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Jan 8, 2026
Document the entire C API #141004
Closed
@StanFromIreland StanFromIreland mentioned this pull request Jan 8, 2026
gh-141004: Improve make check-c-api-docs #143564
Merged
ZeroIntensity
ZeroIntensity approved these changes Jan 8, 2026
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

encukou reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jan 8, 2026
@encukou

encukou commented Jan 9, 2026

Copy link
Copy Markdown
Member

@webknjaz, does this look interesting?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@webknjaz

webknjaz commented Jan 9, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland
PR suggestions
42c0869
@StanFromIreland

StanFromIreland commented Jan 9, 2026

Copy link
Copy Markdown
Member Author

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 :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

webknjaz
webknjaz reviewed Jan 9, 2026
View reviewed changes
Comment thread .github/workflows/reusable-check-c-api-docs.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland @webknjaz
Update .github/workflows/reusable-check-c-api-docs.yml
4ca4198
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
webknjaz
webknjaz reviewed Jan 10, 2026
View reviewed changes

@webknjaz webknjaz left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread .github/workflows/build.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Jan 12, 2026

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland

StanFromIreland commented Jan 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.

encukou reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland

StanFromIreland commented Jan 12, 2026

Copy link
Copy Markdown
Member Author

I can open a PR on your fork against your branch with the changes when this is merged.

encukou reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou
encukou merged commit e5b5a15 into python:main Jan 13, 2026
45 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 13, 2026
@encukou

encukou commented Jan 13, 2026

Copy link
Copy Markdown
Member

Thank you!

Feel free to send PRs to my branch. Or merge to your branch and send the CPython PR from that.

StanFromIreland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland
StanFromIreland deleted the check-c-api-docs_GHA branch January 13, 2026 13:20
@webknjaz

webknjaz commented Jan 13, 2026

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Jan 13, 2026

Copy link
Copy Markdown
Member

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?)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@webknjaz

webknjaz commented Jan 14, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@webknjaz webknjaz moved this to 🌈 Done 🦄 in 📅 Procrastinating in public 😵‍💫 Jan 14, 2026
@webknjaz webknjaz added this to 📅 Procrastinating in public 😵‍💫 Jan 14, 2026
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
@StanFromIreland @webknjaz @thunder-coding
pythongh-141004: GHA: Run check-c-api-docs check on docs-only PRs (p…
c2fa2d5
…ythonGH-143573)

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@ZeroIntensity ZeroIntensity mentioned this pull request Jul 19, 2026
Check for undocumented C API failing (PyException_HEAD) #154058
Closed
@bedevere-app

bedevere-app Bot commented Jul 22, 2026

Copy link
Copy Markdown

GH-154464 is a backport of this pull request to the 3.14 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

StanFromIreland added a commit that referenced this pull request Jul 24, 2026
@StanFromIreland
[3.14] gh-141004: GHA: Run check-c-api-docs check on docs-only PRs (G…
a722c8d
…H-143573) (#154464)

(cherry picked from commit e5b5a15)
StanFromIreland added a commit that referenced this pull request Jul 24, 2026
@miss-islington @StanFromIreland
[3.13] gh-141004: GHA: Run check-c-api-docs check on docs-only PRs (G…
af04851
…H-143573) (#154640)

(cherry picked from commit e5b5a15)

Co-authored-by: Stan Ulbrych <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@webknjaz webknjaz webknjaz left review comments
@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner
@hugovk hugovk Awaiting requested review from hugovk hugovk is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@StanFromIreland @encukou @webknjaz @hugovk @ZeroIntensity
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.