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.
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.
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.
The check_next comment seems to be good OTOH.