Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-141004: GHA: Runcheck-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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
encukou merged 3 commits intopython:mainfromStanFromIreland:check-c-api-docs_GHA
Jan 13, 2026

Conversation

@StanFromIreland
Copy link
Member

@StanFromIrelandStanFromIreland commentedJan 8, 2026
edited
Loading

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 thecheck-generated-files job, but that would make it quite messy as all the other checks would have to be excluded in the case when onlyrun-docs is true. And, to avoid having to configure, I run the script directly (since it usesPYTHON_FOR_REGEN anyway, there shouldn't be a difference).

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

Copy link
Member

@ZeroIntensityZeroIntensity left a 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 reacted with thumbs up emoji
@encukou
Copy link
Member

@webknjaz, does this look interesting?

@webknjaz
Copy link
Member

webknjaz commentedJan 9, 2026
edited
Loading

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 myreusable-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 areusable-*.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
Copy link
MemberAuthor

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>
Copy link
Member

@webknjazwebknjaz left a 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
Copy link
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 theDoc/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.)

@StanFromIreland
Copy link
MemberAuthor

StanFromIreland commentedJan 12, 2026
edited
Loading

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

@StanFromIreland
Copy link
MemberAuthor

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

encukou reacted with thumbs up emoji

@encukouencukou merged commite5b5a15 intopython:mainJan 13, 2026
45 checks passed
@encukou
Copy link
Member

Thank you!

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

StanFromIreland reacted with thumbs up emoji

@StanFromIrelandStanFromIreland deleted the check-c-api-docs_GHA branchJanuary 13, 2026 13:20
@webknjaz
Copy link
Member

Here's the code to parse Intersphinx inventory:main...encukou:cpython:doccheck-intersphinx

Orfrom 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
Copy link
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 overzlib

(Are you sure your site works with the latest Sphinx?)

@webknjaz
Copy link
Member

webknjaz commentedJan 14, 2026
edited
Loading

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 (everythree 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!

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

Reviewers

@webknjazwebknjazwebknjaz left review comments

@ZeroIntensityZeroIntensityZeroIntensity approved these changes

@ezio-melottiezio-melottiAwaiting requested review from ezio-melottiezio-melotti is a code owner

@hugovkhugovkAwaiting requested review from hugovkhugovk is a code owner

@AA-TurnerAA-TurnerAwaiting requested review from AA-TurnerAA-Turner is a code owner

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@StanFromIreland@encukou@webknjaz@hugovk@ZeroIntensity

[8]ページ先頭

©2009-2026 Movatter.jp