fpagliughi / sockpp

Modern C++ socket library.
BSD 3-Clause "New" or "Revised" License
782 stars 126 forks source link

Doc-comment issues (Clang warnings with -Wdocumentation) #16

Closed snej closed 5 years ago

snej commented 5 years ago

There are a number of glitches in the doc-comments, especially @param directives, like name mismatches and empty descriptions. They're flagged as warnings by Doxygen, and by Clang if -Wdocumentation is on.

Not a big deal, except that the project I work on builds with -Wdocumentation -Werror (because it's a library with its own API and doc-comments), so when I include sockpp headers they break the build.

I can work around this by wrapping all the includes with #pragma clang diagnostic push / ignored / pop, but it's a pain and it'd be nice if these were cleaned up along with the other header-file work that's acknowledged in the README :)

fpagliughi commented 5 years ago

Did you try this with the latest from the repo? I fixed many of the Doxygen warnings a few days ago with commit 1726e6e9fd0a41006e239e84af3e16d9d5461405. There were still a couple concerning unresolved @ref references, and I wasn't sure why they were unresolved.

snej commented 5 years ago

I hadn't merged with the latest master yet. Afterwards, there seems to be only one left, at sock_address.h:136 -- "Empty paragraph passed to @param command"

    /**
     * Copies another address to this one.
     * @param addr
     */
fpagliughi commented 5 years ago

As a rule of thumb, I don't know that I would set up my build so that it could be broken by the comments in a 3rd Party, open-source library!

Certainly I mean to keep the docs up-to-date, but even a quick look at the line you mentioned, and I noticed that the two methods above it aren't properly documented either - they're missing the params entirely:

/**
 * Constructs an address.
 */
sock_address_any(const sockaddr* addr, socklen_t n) {
    ...
}
/**
 * Constructs an address.
 */
sock_address_any(const sockaddr_storage& addr, socklen_t n)  {
...
}

So I'll make another pass at it, and fix the one mentioned. But that doesn't mean that I won't break it again in the next commit.

I don't use clang as my default compiler, but have a few versions set up in tests and CI. Maybe I can set up the warnings to at least alert me when I run the build tests.

fpagliughi commented 5 years ago

OK. I did the following in b0cbf7ae4a0701c0f9cbe47ce83f7051e08f9eb1:

snej commented 5 years ago

As a rule of thumb, I don't know that I would set up my build so that it could be broken by the comments in a 3rd Party, open-source library!

Yeah, I know :/ But the alternative is to wrap all #includes of 3rd party headers with several lines of #pragma clang diagnostic ... boilerplate. It's easier just to whine to the developers ;-)

the two methods above it aren't properly documented either - they're missing the params entirely

Clang doesn't warn if all of the params are undocumented.

Thanks for being accommodating!