Hi Jakub,
Thanks for your swift response :)!
On 07/20/2016 04:17 PM, Jakub Hrozek wrote:
On Wed, Jul 20, 2016 at 03:53:54PM +0300, Nikolai Kondrashov wrote:
> I suggest several measures which could be taken.
>
> 1. Require *every* new module/function/argument/structure/field to be
> documented. It must be at least a sentence for each. A module or a bigger
> data structure could warrant more. Describe module/function/structure
> overall, and each of its declarations/arguments/return values/fields.
I'm OK with documenting interfaces and concepts, but not OK with documenting
every function. I would rather spend time on making the code more readable
than effectively creating two places to maintain (the code and the
docs). Especially for static functions, I really don't see the value.
So I'm fine with adding documentation to headers, but I think it's
busywork to add doxygen headers to every function.
I also think it's OK to document why the code does what it does,
but
documenting private interfaces wouldn't help in my opinon, but adding
more comments to the code does.
Documenting private interfaces is basically the same as documenting the code,
and serves the same purpose: making it easier to understand the code. When
you document an interface (public or private, doesn't matter) you narrow it
down in people's understanding, making it easier to reason about. Besides,
today's private interface is tomorrow's public interface and vice versa.
Furthermore, regarding busywork, if a function/structure/module is simple,
then it's easy to document. If not, then it surely deserves the effort of
documentation, and perhaps even refactoring.
Yes, documentation is duplicated knowledge, but it saves time and is worth the
effort, considering the old fact that code is more often read than written.
All that said, even just the public interfaces is much better than the current
situation. I, personally, will be happy to see that :)
> 2. Next level, documentation tax: if you touch anything, document
it. Modified
> a module header? Add a short module description. Added or removed a field
> in a structure? Add structure and field descriptions. Changed a function?
> Document it.
better: If a developer works on a module that is confusing to
understand, document the code flow.
And here is the pitfall: the author is the one least qualified to judge how
confusing the code is, because he/she already groks it. I'm not saying it's
impossible, but bias would be expected.
> 3. Third level, documentation duty: document a whole existing
module header of
> your choosing, say once a month.
>
> The format of documentation doesn't matter, as long as it's legible and
agreed
> upon. I suggest Doxygen, but it can be anything else. We don't have to
> actually run the documentation extraction tool (such as "doxygen") on our
code
> and use perfectly correct syntax (although that could be the next step). What
> matters is that documentation is there, it's uniform, and you don't have to
> think about the format when writing it.
>
> For example of how it can look see the attached patch where I tried to
> document some things (likely misunderstanding some) as I read the code.
Some pieces of the patch are beneficial, some are not. I don't really see
how documenting that pvt_ctx is a private context improves the code. That's
obvious from the name of the structure member, if not, we need to rename
the member. If the patch added something like "this is a context that
holds private data such as NSS context that contains NSS client data etc
for NSS responder" if would be much better.
Thanks, I'll add that!
The check_next comment seems to be good OTOH.
Ah, good.
Otherwise, this patch was just an illustration, I'll be sending it as part of
my SSSD/tlog integration patchset if we agree on documentation in general and
particular format. If not, I'll just drop it. Let's not discuss it here, but
thanks for the comments :)
Nick