Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 commentedJan 9, 2026
@webknjaz, does this look interesting? |
webknjaz commentedJan 9, 2026 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 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
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 commentedJan 9, 2026
Done :-) |
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
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.
Uh oh!
There was an error while loading.Please reload this page.
encukou commentedJan 12, 2026
As I replied to the comment mentioned in the OP:
Here's the code to parse Intersphinx inventory:main...encukou:cpython:doccheck-intersphinx Would it be possible to hook up GHA so that the |
StanFromIreland commentedJan 12, 2026 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 commentedJan 12, 2026
I can open a PR on your fork against your branch with the changes when this is merged. |
e5b5a15 intopython:mainUh oh!
There was an error while loading.Please reload this page.
encukou commentedJan 13, 2026
Thank you! Feel free to send PRs to my branch. Or merge toyour branch and send the CPython PR from that. |
webknjaz commentedJan 13, 2026
Or |
encukou commentedJan 13, 2026
(Are you sure your site works with the latest Sphinx?) |
webknjaz commentedJan 14, 2026 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yeah, I was lazy and didn't pin versions. So it always pulls whatever's latest on PyPI (every |
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.