-
Notifications
You must be signed in to change notification settings - Fork 482
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
Revise Plugins API, add plugins
keyword arg for makedocs
and doctest
#2249
Conversation
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 looks great, thank you @goerz! Other than being conservative about what kwargs to pass from doctest
-> makedocs
, the other comments are purely cosmetic.
Instead of passing plugin objects to `makedocs` as positional arguments (which was undocumented), they are now passed as a a keyword argument `plugins`. The docstrings of the `Plugin` type and the `getplugin` function have been corrected. They were at best misleading, and at worst flat-out wrong.
Forward keyword arguments from `doctest` to `makedocs`. This includes the `plugins` keyword argument, but potentially other keyword arguments as well.
Ok, I made all the changes from @mortenpi's comments. Should be ready for merge, unless there's any more changes related to the revised more conservative approach of only passing |
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, thanks! I'll leave it open for a day or two, in case there is any other feedback, but otherwise I'm be happy to merge this as is.
Plugin
/getplugin
API (testing all documented behavior)Plugin
/getplugin
. Specifically, the documentation claimed that it was possible to passPlugin
-types instead of objects tomakedocs
. This didn't match either the old or the new implementation, so the docstrings have been corrected to match what is actually implementedplugins
keyword argument tomakedocs
that replaces passing instances ofPlugin
as positional arguments (which was completely undocumented)plugins
keyword argument todoctest
to address doctest and CitationBibliography DocumenterCitations.jl#34.This is implemented by forwarding all unknown keyword arguments fromdoctest
to the internal call ofmaketest
. The feature is documented as "use only as directed". Alternatively, I could implement this by fowarding onlyplugins
, but generally forwardingkwargs
seems more future-proof, avoid problems where in some specific contextmaketest
requires some argument thatdoctest
isn't giving it.The introduction of the
plugins
keyword argument is (at least arguably) a breaking change, and thus should be included in the 1.0 release. The old behavior with regards to plugins wasn't documented, but anyone who might have been passingPlugin
objects as positional arguments tomakedocs
has to change that to aplugins
keyword argument.Most notably, users of DocumenterCitations will be affected by this.
In any case, this PR makes the API for plugins significantly more explicit.
Closes #2245