I agree that readability is a huge factor in code quality and is a key to its survival. But I tend to disagree that a commit should be rejected based on this criteria alone. IMHO the low bar for committing to trunk should be that it compiles, does no harm, and has undergone adequate discussion within the dev community. Providing adequate comments lies just outside that threshold as something that you should expect to be nagged heavily about but not necessarily vetoed. Like you say, in most cases this will be resolved through simple discussion.
Best wishes, Paul On 9/15/06, Kevan Miller <[EMAIL PROTECTED]> wrote:
During the recent discussions regarding the Geronimo Development process, several people expressed some concern about moving away from RTC. The biggest issue seemed to be that RTC insured multiple people reviewed new code. Having reviewed the code, the reviewers now understood and would be able to support the code (i.e. fix bugs). This is certainly a valid concern. However, even though we're now following CTR, we all need to be making a concerted effort to provide the same level of review as commits are made. No matter what process we're following, IMO, the best way to insure that people are reading *and* understanding your code is to write code that is easy to read and understand. This does not mean writing simple code. It simply means keeping the reader in mind and trying to make their job easier. The single, most important thing, in my mind, is to provide clear and insightful comments to assist the reader. These don't need to be verbose tomes. They don't need to state the obvious. However, any assistance you can provide the reader is helpful. Describe the processing flow that methods are being invoked. What are the threading assumptions? Identify subtleties that a reader might not be aware of. Who are the potential callers? etc... In case anyone is wondering -- I think we've been lacking in this department. I'd like to see simple comment guidelines incorporated into the Documentation Guidelines for our CTR process. In my opinion, failure to appropriately comment new code is cause for a commit to be vetoed. I doubt that this will happen often. I expect that in most instances these issues can be resolved appropriately through simple discussion. Some basic rules-of-thumb are likely to help resolve any issues. What do others think? Specific ideas on comment guidelines? Javadoc-style comments for APIs/ SPIs, etc? What types of comments should be expected? --kevan
