Conversation
as per discussion in #128, changing the deploy directory to a required argument. the new usage for doctr on the travis side of things would been ``` doctr deploy . ``` to deploy to the root directory on the ``gh-pages`` branch.
|
Let's check all known users of doctr to make sure this doesn't break them. |
|
I think it will break all users -- I guess we could PR appropriate changes to them? |
| deploy_parser.add_argument('--built-docs', default=None, | ||
| help="""Location of the built html documentation to be deployed to | ||
| gh-pages. If not specified, Doctr will try to automatically detect build location""") | ||
| deploy_parser.add_argument('--gh-pages-docs', default='docs', |
There was a problem hiding this comment.
Maybe we could keep and deprecate this option for backwards compatibility.
There was a problem hiding this comment.
I was thinking of that, but I'm not sure how to go about having a default argument not be required if a certain non-default option is passed.
There was a problem hiding this comment.
Does argparse let you set a default for positional arguments? You'd have to do the flag logic manually (use parser.error to print the error).
|
I guess we could deprecate it with some custom argparse logic (require the argument if the flag isn't there, and if the flag is used print a deprecation warning). OTOH, deprecation warnings will be ignored, so at best it would just make a transition possible without a period of non-building docs. |
|
There's definitely a benefit to having a period in which to help people transition, I guess I'll see how difficult it is to implement the custom argparse stuff |
|
Yeah, even if we PR everyone, they would all have to merge their PRs exactly when we release, or else their docs will fail. |
|
I was thinking it might work by setting a |
|
Well, you have to set a default value, but it will still require the argument to be present. You need both IIRC. |
to make sure that both methods still work on travis, keeping one of these deploy calls using the old syntax.
|
Ok -- this is ready for another look. @scopatz was (unsurprisingly) right about |
|
I pushed a change to the deprecation message. |
|
Am I missing something, or does this not actually use the deploy_directory variable? It should use it, or |
Nope -- mea culpa. That's a bit of an oversight. I'll add that in later tonight. |
print deprecation warning if `--gh-pages-docs` is used.
|
This is ready for another look. |
|
|
||
| Current | ||
| ======= | ||
| - Change deploy directory to required argument. This is a backwards incompatible change. Default deploy without arguments should now read ``doctr deploy .`` (:issue:`128`) |
There was a problem hiding this comment.
This should be reworded a bit. The default used to be docs, but now there is no default. Maybe something like
There is no longer a default deploy directory. Specify the deploy directory like
doctr deploy .ordoctr deploy docs.
|
The code looks good. Maybe we should test that it works in Travis? Either way, let's check which known repos using doctr need to be updated. |
|
Updated the changelog. The list of repos I have using doctr (searching for
|
|
I will fix sympy. |
|
We should make a new issue about the updates. We can't do it until this is merged and released. |
|
gallantlab/pycortex#205 will switch to doctr (not merged yet) , they should be warned also. |
|
Ok, anything left on this? We're tracking the people we need to notify about the usage changes. This will also require a few changes to @Carreaus' #137 if this is merged first, but those should be trivial. |
+1
No problem, I can take care of that. |
|
I think we should try to get the other PRs in before we release. But this looks good. |
4 participants
as per discussion in #128, changing the deploy directory to a required
argument. the new usage for doctr on the travis side of things would
be
to deploy to the root directory on the
gh-pagesbranch.