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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
