Update various documentation comments #4185
Conversation
|
Feel free to take over this PR/open a new one with this as the base/force push/whatever |
|
Oh wow! Thanks for taking that apart. I'll check it out soon. Agreed though, if breathe is abandoned by now, that's no good. 😔 |
src/lib/tls/credentials_manager.h
Outdated
| * @deprecated This virtual function is deprecated, and will be removed in | ||
| * a future release. Use (and override) find_cert_chain() | ||
| * instead. |
There was a problem hiding this comment.
Clang warns that the docstring claims a deprecation but there's no [[deprecated]] attribute in the function's declaration.
This would be fixable by adding BOTAN_DEPRECATED(). However, when we build with the deprecation annotations being disabled (e.g. amalgamation builds), this clang warning might still show up and break things (due to -Werror).
I think we should either disable this clang warning (-Wdocumentation-deprecated-sync), or not use @deprecated in the Doxygen comments. This seems to be the only/first location where we use it anyway.
There was a problem hiding this comment.
That said: clang has a point. We should actually mark this as deprecated in favor of find_cert_chain(). When invoked in a client-auth context, find_cert_chain() conveniently delivers information about the advertised acceptable CAs of the server.
There was a problem hiding this comment.
I removed the @deprecated comment but also did introduce a BOTAN_DEPRECATED annotation here.
|
Once this is merged, I'll close the various breathe PRs to reduce clutter. I agree that breathe doesn't seem to deliver on what we need. Nevertheless, it would be great to somehow interface between the sphinx and doxygen documentation to avoid duplicating too much content. |
randombit
force-pushed
the
jack/breathe-docs
branch
from
e9c10cb
to
b9c172f
Compare
I do agree, there is a real need for something that is basically Doxygen+Sphinx combined (but ideally also faster than either...) |
This was originally done by René in a series of doc updates related to using Breathe for our API reference. I (Jack) am still unconvinced of adopting this for various reasons (build times, limited distro support, apparently abandoned upstream, etc) but the documentation comments themselves are certainly an improvement.
randombit
force-pushed
the
jack/breathe-docs
branch
from
8e7fe95
to
2b46eab
Compare
@reneme This is a merge of (as best I could handle after the various merge conflicts) the doc updates you made for Breathe. I'm pretty unconvinced of Breathe still (and especially since it seems to have been abandoned by the upstream dev now...) but the doc comments obviously have a lot of value no matter what. In retrospect I wish we had merged the doc comment updates immediately at the time, and deferred just the Breathe integration part, but so it goes.