Update for Documenter.jl v1 #683
Conversation
docs/src/user/api.md
Outdated
|
|
||
| ## Other API | ||
| ```@docs | ||
| ForwardDiff.can_dual | ||
| ForwardDiff.:≺ | ||
| ``` |
There was a problem hiding this comment.
The docstring for can_dual is not useful at all, and both functions are not exported. IMO including them in the docs should be discussed and proposed in separate issues (#682) and PRs, but not be part of upgrading to Documenter v1.
There was a problem hiding this comment.
Documenter.jl v1 has warnonly=false keyword argument in makedocs. (JuliaDocs/Documenter.jl#2194), so makedocs will fail without adding the docs or setting warnonly=true.
These functions are already documented as docstrings, and I thought it would be okay to add them to the documentation. I understand these functions are not stable.
Should I replace "Other API" with "Internal API"?
There was a problem hiding this comment.
AFAIK you can set checkdocs = :exports.
There was a problem hiding this comment.
Yeah, that's right.
My point is, that the ignored docstrings may confuse other users. The functions are documented as docstrings, so someone may think these functions are stable. I thought emphasizing that these functions are not stable would help that kind of people.
There was a problem hiding this comment.
But they are not exported, are they?
There was a problem hiding this comment.
To clarify, I think these functions should not be included in the documentation for the time being and only added if they are considered important enough and their docstrings are more useful.
There was a problem hiding this comment.
Sorry, I don't quite understand.
Documentation should be written for both package users and developers, and including Internal API documentation will be useful for future package developers. Of course, if these docstrings are incorrect or meaningless, they should be removed, but that is not the case.
There are more functions and types that need to be written (#682), but I believe this is simply a matter of insufficient documentation, and not a reason to eliminate existing documentation.
There was a problem hiding this comment.
While I am not the maintainer of the repository, I am open to following your guidance as the maintainer. If you believe that removing these documents is the best course of action, I am willing to proceed with that.
There was a problem hiding this comment.
AFAICT neither users nor developers should have to deal with can_dual and ≺, so even though there is a docstring I don't think it should be included in the documentation (to me it seems these are just more detailed comments). value, Dual, etc. are more relevant IMO: #682 (but this should be discussed and addressed separately)
There was a problem hiding this comment.
Okay, I'll update this PR to remove them from the documentation.
I will write down additional my concerns for future reference:
- If we use
checkdocs=:exports, that will be as harmful aswarnonly=truebecause this package provides non-exported stable API. - I agree that the docstrings are something like detailed comments. But it would be very hard to discuss whether new docstrings should be in the docs or not every time. The easiest way would be to document the all docstrings.
The doc build is not failing, but I agree with adding |
…uggested by the reviewer
hyrodium
force-pushed
the
docs/documenter_v1
branch
from
0486391 to
675b191
Compare
hyrodium
force-pushed
the
docs/documenter_v1
branch
from
c51f7b4 to
675b191
Compare
* ignore `docs/Manifest.toml` * add missing docstrings * add compat table for Documneter.jl * rename Other API with Internal API * Use `checkdocs=:exports` and ignore internal docstrings as strongly suggested by the reviewer
* ignore `docs/Manifest.toml` * add missing docstrings * add compat table for Documneter.jl * rename Other API with Internal API * Use `checkdocs=:exports` and ignore internal docstrings as strongly suggested by the reviewer
No description provided.