IF its a comment that slows down a patch being accepted then I vote -1 IF comment rules reduces the number of comments in code (because its easier not to comment than to follow the rules) I vote -1

We learned to cope with code style and it greatly enhances the quality of Drupal. If we add comment rules to code style we will cope with that too. Yes sometimes it requires another roll of a patch but this is _not_ a hindrance, much rather a benefit to Drupal. Regards NK Ps. Angie, you rock. As usual.

-1 for proposal, +1 for some guidelines I'm for: * Full sentences and sentence capitalization for comments that stand by themselves, such as the header /* */ comments and larger asides on what's going on * For C++ style comments that stand on a line by themselves, Full sentences not required, but capitalization is * For C++ style comments that are part of another line, anything goes * All caps is OK for some predefined important names such as. But these should always be followed by full sentences and sentence capitalization. I think we should standardize what these names are (NOTE/WARNING/HELP), so one could easily grep for them. I use NOTE and WARNING a lot. HELP could be pretty useful in an open source community project for people to point out parts that are broken and need some help. I also use KLUDGE (others use HACK), although I'm less tied to this. For example: /** * This should always be a full sentence and sentence capitalized. */ // Not a full sentence, but capitalized $var = 'something'; // anything goes // NOTE: Do it this way for performance reasons. // WARNING: Do not change this. // HELP: Code below is broken . My reasoning: Personally, I use the C++ style comment // more for short- quips about what's going on, and the C style comment /* */ for larger asides on the purpose of things. I'm not consistent with this, but I think it would be a good standard. C++ style comments then are helpful hints as opposed to full tutorials. Also, it's nice if the fits on a single line for readability, and I frequently shorten my comments to fragments for this reason. A standard that encourages me to use full sentence structure will result in longer comments cause lines to wrap, and making them less readable. (I don't have an editor that wraps line prettily.) Doug Green www.douggreenconsulting.com www.civicactions.com Changing the world one node at a time! -----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Angela Byron Sent: Sunday, January 07, 2007 12:27 AM To: development@drupal.org Subject: Re: [development] QA: Consensus on comment formatting

So! Anyone have a problem with these rules:

Gah! I copy/pasted the wrong chunk. :P These! :) * Comments are full sentences ending in periods, and are on their own line unless they are documenting lists. * There is no blank line between %param and %return sections in function documentation. * Function documentation begins with a single sentence in the form "Do such and such." rather than "Does such and such.", and has a blank line before any further comments. * @file documentation begins with a sentence in present tense, and has a blank line before any further comments. * Eliminate inappropriate uses of all caps, e.g, NOTE. * Spelling, grammar, punctuation corrections.

-// Clean up +// Clean up. I like the period at the end. Agree on that.

* Eliminate inappropriate uses of all caps, e.g, NOTE. Think we should allow NOTE and WARNING as long as they are used like TODOs -- with a capitalized sentence/period. All these are good in my mind.

// TODO: Optimize this query. // WARNING: Don't do that. // NOTE: May smell funky. That is all. Rob Roy Barreca Founder and COO Electronic Insight Corporation http://www.electronicinsight.com rob@electronicinsight.com Angela Byron wrote:

On 7-Jan-07, at 9:05 AM, Doug Green wrote:

...

-1 for proposal, +1 for some guidelines

I'm for:

* Full sentences and sentence capitalization for comments that stand by themselves, such as the header /* */ comments and larger asides on what's going on * For C++ style comments that stand on a line by themselves, Full sentences not required, but capitalization is * For C++ style comments that are part of another line, anything goes

If you look at Nedjo's patch, the "full sentences" is purely about the format of the comments, not subject-predicate agreement. For example:

-// Clean up +// Clean up.

I agree we shouldn't have to write:

// Now, clean up the various variables related to the cron job.

But, it just looks really sloppy to have comments like:

// Clean up // clean up // Clean up. //clean up .

... all over the place. We should pick one way of formatting and standardize on it, and "space after the comment delimiter, starts with capital letter, ends in period" is the most consistent.

...

* All caps is OK for some predefined important names such as. But these should always be followed by full sentences and sentence capitalization. I think we should standardize what these names are (NOTE/WARNING/HELP), so one could easily grep for them. I use NOTE and WARNING a lot. HELP could be pretty useful in an open source community project for people to point out parts that are broken and need some help. I also use KLUDGE (others use HACK), although I'm less tied to this.

Sure, I could go either way on this. NOTE, TODO, etc. are handy clues that you should pay attention. Core just was using "Note:" in some places and "NOTE:" in others, so Nedjo was trying to find a consistent way to show this. I think as long as caps aren't used excessively, this is fine for a few keywords.

-Angie

Quoting Angela Byron <drupal-devel@webchick.net>:

Sure, I could go either way on this. NOTE, TODO, etc. are handy clues that you should pay attention. Core just was using "Note:" in some places and "NOTE:" in others, so Nedjo was trying to find a consistent way to show this. I think as long as caps aren't used excessively, this is fine for a few keywords.

To me when I read "Note:" it tells me that it is something the developer added to the comment section to remember but isn't necessarily important to further development; but, when I read "NOTE:" it tells me that the developer wanted to remember something that is very important to further development and that I shouldn't ignore it. I don't have any such cognitive idea about "TODO" vs "Todo" but Todo isn't an officially defined word and should be capitalized. I do agree that too much capitalization is the wrong thing but should be used to the benefit of others. Earnie

I think this is very helpful. I have made a few minor comments on the existing proposed rules at the bottom. One other area that is inconsistent is the use of quotes around table names, variables, code snippets, URLs and file names. I suggest that for simplicity and brevity the rule should be: Table names, variables, code snippets, URLs, file names etc. should not be enclosed in quotation marks (") or inverted commas (') unless it significantly enhances the readability of the comment for other developers. Where necessary, inverted commas should be used rather than quotation marks. Hopefully some examples will help people decide what they like. From bootstrap.inc: // they come first, and we return NOT(status) = 0 (allowed). Otherwise, ... versus ... // The use of ORDER BY / LIMIT is more efficient than "MAX(status) = 0" --- I don't think the quotes in the second line make it clearer than the first. * Role ID for authenticated users; should match what's in the "role" table. ... versus ... * The variable table is composed of values that have been saved in the table ... versus ... // We deny access if the only matching records in the {access} table have --- Again the quotes do nothing for me. Where a table name is used several times in a sentence in a block comment, repeatedly quoting "tablename" would look clunky. The third option here at least has technical merit and would be the best way to delimit table names if it is thought necessary, but still seems like overkill. Table names do not contain spaces and so are already delimited. * Authenticated users are always given a 'no-cache' header, and will ... versus ... // All 304 responses must send an etag if the 200 response for the same object contained an etag --- The first is not clearer than the other, assuming a basic knowledge of HTTP. * configuration file is found, return a default value '$confdir/default'. --- To me is is surprising to see a variable in '', and no clearer than leaving them out. Other notes: - I agree with Doug that the full sentences rule could be overkill in some contexts. One example might be single line comments introducing if, else options. - TODO: comments are a common semi-standard where the captilisation is useful. It would be helpful to specify whether TODO should be interpreted as excessive. Alternatively, if an api.d.o like site were planned for contrib, developers could in future be advised to use the doxygen commands @todo, @warning or @note. Sadly @hack is not implemented ;) - An effort to keep lines to 80 characters would be helpful. - On grammar, is it fair assume the standard is clarity to another English-speaking developer (e.g. there and their)? If I want to start a comment with a conjunction, (such as before an else{}) that shouldn't be an issue. - Might be worth mentioning that Drupal documentation should use US spellings ('color' rather than 'colour').

+1 for for Angie and Nedjo calling for a standard. ;) +1 for standardizing on @todo, @note, etc as supported by phpdoc/ doxygen[1]. +1 for a newline between @param and @return. +1 for allowing // A little extra clarity. (as a "1-liner") +1 for formatting comments like sentences, but not requiring full sentence grammar on 1-liners. +1 for 80 chars wide if possible. +1 for always referring to DB table names as {foo}, not "foo", 'foo' or foo. Even though it's technically not required as a delimiter since the names can't have spaces, i find it adds a lot of clarity to quickly be able to tell if a comment is talking about the DB at all, and if so, which table(s). Here's a new draft of the conventions given everyone's input, my votes above, and a few other suggestions: * Comments should be written in English, with correct spelling[2], punctuation, and grammar. * Being concise is best, but never at the expense of clarity. * Comments are capitalized and end in periods. They should be complete sentences on their own line unless they are documenting lists, or are describing a line of code that only needs a few extra words to be clear. sentences are separated by a single space.[3] * @file documentation begins with a sentence in present tense, and has a blank line before any further comments. * Function documentation begins with a single sentence in the form "Do such and such." rather than "Does such and such.", and has a blank line before any further comments. * There is a blank line between @param and @return sections in function documentation. * Comments should not use all caps, e.g, NOTE. Instead, use @note, @todo, @warning, @bug, etc. * Any comment that refers to a variable should include the dollar- sign in front of the $variable_name. * Any comment that refers a database table should use {table_name} to make it visually unique, not "table_name", 'table_name', etc. * In general, URLs, code snippets, file names, etc, should not be enclosed in quotation marks (") or inverted commas (') unless it significantly enhances readability. Where necessary, inverted commas should be used rather than quotation marks. * No comment should be, or cause a line of code to become, wider than 80 characters. * In general, comments should not refer to issue node ids (for example, "// This is crazy, and we fixed it in #12345."), since that's what the CVS annotation and commit logs are for. However, links to issues are allowed (recommended?) in a @todo or @bug. In this case, a full URL should be used. For example: // @todo These queries are very expensive, see http://drupal.org/ node/105639. any final changes/editions/subtractions? ;) thanks, -derek [1] http://www.stack.nl/~dimitri/doxygen/commands.html [2] i really don't care about "american" vs. "british" spelling. english sux, but it's the best we've got. :( [3] personally, i find 2 spaces between sentences *way* more readable, but i've already lost that debate (at least a bunch of patches already went into core to force it to 1 space) so i don't intend to re-hash that decision. i'm now in favor of 1 space in comments, just for consistency and less work moving forward.

Quoting Derek Wright <drupal@dwwright.net>:

[3] personally, i find 2 spaces between sentences *way* more readable, but i've already lost that debate (at least a bunch of patches already went into core to force it to 1 space) so i don't intend to re-hash that decision. i'm now in favor of 1 space in comments, just for consistency and less work moving forward.

Yes, me too. Frankly I dispise HTML rendering engines for this; I put two spaces in, I would like it rendered two spaces. But, anyway, +1 for Derek's summarization of items. Earnie