Compatibility with Sphinx 8.2, minor clean up #196
Conversation
|
wow I made it worse lol |
|
Ahh, I started working on this just this morning, but haven't got far so will abandon the effort :) |
|
Ooops sorry for the duplicated work, @bsipocz ! |
|
Should use the "squash and merge" button, or if you want me to manually do it, pls lemme know. Thanks! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #196 +/- ##
==========================================
- Coverage 90.29% 90.09% -0.21%
==========================================
Files 30 30
Lines 1206 1211 +5
==========================================
+ Hits 1089 1091 +2
- Misses 117 120 +3 ☔ View full report in Codecov by Sentry. |
|
No worries about duplicates, I only started digging. Actually, I appreciate you opening a draft early on, so we in fact avoided doing much duplicates. |
| return old_generate_dot(self, name, urls=fullname_urls, env=env, | ||
| graph_attrs=graph_attrs, node_attrs=node_attrs, edge_attrs=edge_attrs) | ||
| if SPHINX_LT_8_2: | ||
| def patched_generate_dot(self, name, urls={}, env=None, |
There was a problem hiding this comment.
While we're at this, if possible, make the kwargs mandatory so the API change won't have any chances to bite us back.
There was a problem hiding this comment.
I added it, though hopefully won't break downstream...
|
Hmm looks like that breaks Sphinx itself... should I revert adding |
|
Ahh, it was worth a try. Go ahead and merge it once CI passes. |
|
Thanks for the review! |
Goal: Fix devdeps CI
I followed diff in sphinx-doc/sphinx#13212