DM-9801: Doxygen warnings in astshim #45
Conversation
There was a problem hiding this comment.
I think I'd approach both of the commits a little differently. Depending on base should suppress undocumented member warnings (and guarantee that astshim documentation behaves like the rest of the Stack in general). On the other hand, I think using EXCLUDE_PATTERNS will have unintended effects on the Stack-wide build, and that letting the source files get processed would be the lesser evil.
doc/doxygen.conf.in
Outdated
| @@ -0,0 +1 @@ | |||
| EXCLUDE_PATTERNS += */src/*/*.cc | |||
There was a problem hiding this comment.
sconsUtils will append this line to builds of the Stack-wide documentation. I don't think the Stack as a whole is ready for excluding source files; it would certainly need to be RFC'd.
I think FILE_PATTERNS doesn't get picked up from packages, so you might try
My mistake, local overrides always take precedence. If you can't express the exclusion in ways that won't collide with other packages, I'd recommend leaving the source parsing in.
include/astshim/Channel.h
Outdated
| Channel(Channel &&) = default; | ||
| Channel &operator=(Channel const &) = delete; | ||
| /// Move assignment operator |
There was a problem hiding this comment.
For move assignment, it might be worth documenting whether or not the class follows the convention of leaving the source object in a valid state; I suspect the answer might be no.
include/astshim/Channel.h
Outdated
| @@ -76,8 +76,10 @@ class Channel : public Object { | |||
| virtual ~Channel(); | |||
|
|
|||
| Channel(Channel const &) = delete; | |||
| /// Move constructor | |||
There was a problem hiding this comment.
The Stack-wide Doxygen config in base (which astshim doesn't depend on) sets EXTRACT_ALL = YES, which disables warnings about undocumented members as a side effect. I'd propose depending on base instead of manually adding no-op documentation.
doc/doxygen.conf.in
Outdated
| @@ -0,0 +1,2 @@ | |||
| EXTRACT_ALL = YES | |||
| EXCLUDE += src | |||
There was a problem hiding this comment.
EXCLUDE will still be copied for cross-package docs. I don't think there's any way to trick lsstDoxygen about that.
Though looking at the Doxygen documentation, EXCLUDE_PATTERNS += */astshim/src/* might do the job. As for afw, we don't care about stuff getting used if it only matches one package...
There was a problem hiding this comment.
That sounds perfect. I also agree that in this case we don't really mind if Doxygen scans the source, but this works and seems safe. I'll use the same pattern for afw as well (where we really do want to stop Doxygen from scanning the source, at least some source).
Use EXTRACT_ALL = YES to stop warnings about undocumented default constructors. Use EXCLUDE to exclude source code, since it's a waste of time. EXCLUDE_PATTERNS seems a bit more obvious, but it gets picked up by the LSST documentation builder and used in all packages).
No description provided.