Hi everyone,
I've been acquainting myself with SSSD code the last few weeks.
Particularly, the NSS responder, and what it uses down the stack.
The SSSD code is sure complex and its asynchronous nature doesn't
help. However, what struck me, and frustrated me the most, as a new
reader of the code, was the meager amount of code documentation.
In the absence of documentation, a piece of logic, such as processing
a getpwnam request, requires traversing many functions, modules, and
several programs. Sure, you can guess what a function does, based on
its name and somewhat on arguments, but you can't be sure without
reading it and the functions it calls, and often the functions those
call in turn. Same goes with data structures: to understand them you
need to read the code that creates and operates them.
To the author code often seems obvious, but a new reader has a dozen
questions: which state the passed object should be in, how does the
function modify the object, where does it put its output, what else
it affects, exactly what string should be that, what status codes the
function can return and what will they mean, etc.
Documentation cements the author's intention in the mind of the
reader, freeing the memory and precious attention for understanding
the code at hand, as opposed to requiring going deeper and deeper
into the underlying code, where the reader eventually forgets why he
or she is looking at it. Reading such complicated code as can be seen
in SSSD without documentation is an exercise in frustration.
We often speak about cooperation and getting more external
contributors. However, we're not going to get many if they have such
a mountain of code to climb without our help. We ourselves will be
(and are) struggling to understand our own code, and will pay with
lost time again and again when trying to modify or extend it.
In short: it is NOT OK that SSSD code has no documentation. Put
bluntly, it is sloppy engineering. I'm sure we can do better.
If you have just a sentence describing each function and each of its
arguments, understanding the code becomes considerably easier. You
don't have to guess that much, and you don't have to verify your
guesses and read so much more. Function documentation would often
make reading its code unnecessary. A sentence for a data structure
and each of its fields makes understanding the code operating it much
easier.
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. 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. 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.
For more examples of what I mean see the tlog header files:
https://github.com/Scribery/tlog/tree/master/include/tlog
They are not the best example and there are many ways they can be
improved (tell me please!), but if SSSD had just that it would be
already awesome.
Please tell me what you think, object, or suggest your own
solutions!
Thank you.
Nick
Hi Nick,
when I joined SSSD I used to document everything, exactly the way you
do. But I was trained by Jakub not to do it and in time I started to see
benefits of documenting only public interface that is bound to be stale.
Maintaining documentation is a pain and core developer doesn't need it
since we can understand most of the code quite quickly and we rewrite
the poor parts we don't.
However, I can see the benefits of code documentation for new comers. So
the question is, would this help us to attract more contributors? Or is
SSSD already too complicated for any ordinary coder to step in? I'm
afraid its the latter, since serious development requires lots of
knowledge in both outside and inside SSSD that simply don't come in a
few days.
Do you have an example of a project that gained more contributors after
creating a documentation?
I personally see more benefit in writing articles on how SSSD works (we
already have some) and on our coding habits (e.g. EAGAIN mean
asynchronous processing) than in documenting code.
Unfortunately, you chose probably the worse code for start that you
could possible choose. NSS responder is one of the oldest code that I
admit is terrible to read and contains lots of per-feature hacks. But as
you dig in, you see that most of the code is just about reading data
from cache and refreshing them in providers. All this complexity can be
now done with one call of cache_req. Since you have the code in grasp,
would you like to convert it to cache_req please?
Pavel.