-
Notifications
You must be signed in to change notification settings - Fork 201
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
Sphinx 7 support #744
Sphinx 7 support #744
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
c94bf43
to
9313f58
Compare
myst-nb actually supports sphinx 7, we simply need the maintainers to tag a new release... |
Indeed, you can see the author there :D executablebooks/MyST-NB#524. Right now Sphinx-thebe is the hold-up executablebooks/sphinx-thebe#67. I don't know how to make a good testing suite there to decouple it from |
Is there any word on this being resolved? |
If we can yolo both this and sphinx-thebe to get rid of the silly cyclic dependency the CIs will then be back running properly and we can pick up from there. @choldgraf any thoughts? |
I've just made a release of sphinx-thebe to unblock this on the version pinning issue: https://github.com/executablebooks/sphinx-thebe/releases/tag/v0.3.0 Myst NB also has a release candidate out (1.0.0rc0) that removes the sphinx pinning issue there too Sorry for the slowness - I or @agoose77 can try to help provide review to get this over the finish line if @LecrisUT has cycles to get the tests passing |
Ok, I'll get the CIs back and running, and mark some of them as experimental if needed, then we can open new PRs to de-mark the experimental. Sounds good? Btw, what are your thoughts on making some of the CIs more templetize (or even importable) to ve consistent across each other. I can write a mock-up over here and then have them moved to a new repo maintained more regularly under executablebooks? |
For now I'd recommend just taking the minimal actions and complexity needed to support the latest Sphinx and be reasonably certain we aren't introducing obvious regressions. Improving broader infrastructure sounds good but like it might add a lot of extra work to getting this unblocked. Right now we are extremely heavily limited in maintainer capacity... |
Signed-off-by: Cristian Le <[email protected]>
for more information, see https://pre-commit.ci
@choldgraf Ok, for now here is some minimal updates, with a few fixed pytests. The currently failing ones seem trivial, just need to update the regression test files for different versions. The windows CI does not work in how I wrote it. Can you help with that? I will mark it experimental for the time-being Sorry a bit slow to push commits because pre-commit is having trouble updating on my side |
for more information, see https://pre-commit.ci
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
All ready on this PR, might add some CI improvements in another PR. The only one I don't understand is the windows one :(. Should be fine for at least a pre-release tag, since |
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.
Looks good to me - let's merge it in and iterate on PRs from there. I'm not sure what's up with the Windows bug either, but I don't think it's related to this PR. Thanks for making this effort!
Thank you both for the hard work! @LecrisUT @choldgraf |
Cannot install testing suite because of dependency hell. Any advice on this? See my fork PR LecrisUT#1. Any ways to drop
myst-nb
andsphinx_thebe
testing requirement? It is hell to try and detangle their testing requirements.