-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Break intersphinx
into a package
#12178
Conversation
I assume that all public functions were re-exported right? |
yes, they are all part of |
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.
Assuming this basically just moves code around (haven't checked), then I don't see any issues.
I haven't checked again but if nothing changed since I reviewed and if everything that was public is still exported at the same location, I'm fine with that. Do you want to make another PR where some objects are deprecated and marked as private in 9.x? |
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
ef2d104
to
13b6700
Compare
@AA-Turner please don't merge this as is yet 😅 , |
I just have; I re-split on your delineations from the current state of intersphinx.py (but kept you as the author / credit via rewriting the commit metadata). A |
intersphinx
into a package
intersphinx
into a packageintersphinx
into a package
Ok cool cheers, I didn't see that, just wanted to quickly write something before the CLI finished running 😄 but yeh, as long as we have double-checked that the new code matches the old, then all good from my end 👍 |
No worries -- better safe than sorry & all that! I've also broken up the commits that reorganise the package so it's easier to verify that everything has been moved correctly -- and I've deferred any style updates until after this PR, again in an effort to properly verify everything is all well. A |
perfect 👌 |
I intend to rebase-merge this as-is, but if you want me to hold off at all I'm happy to do so @chrisjsewell |
No thats great thanks, go ahead! |
This change completely maintains the public API and includes no changes in code logic
Internally though it split the code into three, independent, concerns:
This makes the logic clearer and more maintainable for future changes
It also remove this module from the
ruff format
exclude list