On Friday, September 14, 2012 2:57:24 PM UTC-7, Justin Lebar wrote: > I propose here that reviewers and super-reviewers should require that > new code be appropriately commented, just as we require that new code > be appropriately formatted. (We seem to be quite good at enforcing > the latter, even though I hope we can agree that comments are, at a > minimum, as important as formatting. Maybe we should put something > about commenting in the style guide. :) > > "Appropriately commented" is obviously up to individual > interpretation. But I think at a minimum, interfaces and classes > defined in header files should be preceded by a sentence or two > explaining their purpose and perhaps how they interact with other > pieces of code. Additionally, I'd say that methods and arguments in > an API (in a .idl, .ipdl, .h, or .js file) should be documented to an > extent which would allow a programmer of average skill (for Mozilla) > to correctly use the API without reading the implementation. That may > mean that none of the methods on a particular API get any comments, > which is totally fine by me.
I also recommend asking for good comments in reviews. I started asking for comments in certain JS patches a while back. I've been doing more of it lately. And I think the experience is pretty good. In particular I remember a case where I said 'put a comment explaining how this works', the patch author added the comment, and about 2 weeks later a contributor wrote a patch and happened to say in the bug 'and there was this nice big comment that explained everything and made this much easier' (paraphrasing). Another benefit is that the effort of writing good comments gets you to think through the ideas clearly, and if they can't be expressed clearly, maybe the code needs revision too. I learned my commenting style from Code Complete. I don't remember exactly what the book said, but my target commenting style is something like: - making the *purpose* of code clear is the most important thing--it's the one thing you really can't express in code - anything that's obvious (from context, convention, conveniently located assertion) is *not* commented - it costs something to have comments, so it's always a judgment call--sometimes if it's only sort of obvious you still don't comment - we also have a domain-specific practice of documenting code with the section of the JS spec that it implements, where applicable - 'modules', 'components', and other major pieces get an explanatory overview comment saying what it's for, how it works, overviews of major cross-cutting invariants and data structures. Example: js/src/vm/Stack.h - classes, structs, and other things that represent key concepts or abstractions get a comment about what the key abstraction is and what it's for. - functions, methods, and other function/command things get a comment saying what it does, what it's for, and how to use it correctly (meaning of arguments, postconditions, preconditions, invariants, temporal constraints, etc) - non-obvious tricks in the code get an explanation But it's probably better to just read the chapter in Code Complete and adapt it to our various needs. API documentation is a harder to write. In general, it's just a ton of work to maintain good API documentation, and so you have to commit to doing it right and keeping it up. I would lean toward avoiding redundant effort, so that if something has API docs (like the JSAPI) then the in-code comments can be leaner. It would be very cool if we could generate API docs automatically from special comments in the code, but I doubt we could get the same quality of API docs we have now with existing tools, so we'd have to invent something. If you really care about having good API docs, I don't think the (significant) effort of maintaining separate API docs manually is excessive. Dave _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
