-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
[MRG+1] Run sphinxext doctests only on CircleCI #8228
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
Conversation
jnothman
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 still think it's worth running test-sphinxext somewhere, but on Circle, perhaps not in the main tests.
655a942 to
65052e1
Compare
Hmm I made the change without being really convinced. IMO we are very unlikely to find problems in sphinx extensions through doctests. |
65052e1 to
a658648
Compare
797e28b to
8328a7f
Compare
Some sphinx extensions import sphinx in their doctests.
8328a7f to
e790fe4
Compare
|
This should be green now, I had to overcome some idiosyncrasies of CircleCI (mostly that each command is run in a separate shell, so you have to reactivate the conda env in each command). |
jnothman
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.
LGTM
TomDLT
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.
LGTM
Some sphinx extensions import sphinx in their doctests.
Some sphinx extensions import sphinx in their doctests.
Some sphinx extensions import sphinx in their doctests.
Some sphinx extensions import sphinx in their doctests.
Some sphinx extensions import sphinx in their doctests.
Building the documentation on CircleCI is a better test. Also doctests are generally not very extensive tests and sphinx extensions indoc/sphinxextare third party libraries and we should not be the ones testing it.sphinx extensions may have dependencies, e.g. sphinx.
Discussed in #8222 (comment). ping @jnothman.