Re: [development] QA: Consensus on comment formatting
Excellent! Thanks for the summary, Derek!!
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.
Now, just one (technical) question. Does this:
// @todo These queries are very expensive, see http://drupal.org/ node/105639.
...actually work and get parsed by doxygen?? I thought doxygen only worked on function headers. This would make @todo less useful, imo, since: // TODO: this can be an expensive query. Perhaps only execute it every x minutes. Requires investigation into cache expiration. if ($user->uid) { db_query("UPDATE {users} SET access = %d WHERE uid = %d", time (), $user->uid); } Tells me that the queries immediately following that line are expensive, vs. /** * Reads a session variable. * * @todo the update query used here can be expensive.... */ function sess_read($key) { Anyone know? Also, as Robert brought up, // TODO, etc. have support in IDEs (Zend, etc.). Can someone check if @todo does as well? -Angie
On 1/8/07, Angela Byron <drupal-devel@webchick.net> wrote:
Also, as Robert brought up, // TODO, etc. have support in IDEs (Zend, etc.). Can someone check if @todo does as well?
Eclipse supports TODO by default, but allows you to configure the token. I forget where that setting is, but I've seen it. Regards, Greg
Angela Byron wrote:
// TODO: this can be an expensive query. Perhaps only execute it every x minutes. Requires investigation into cache expiration.
While I agree that there are places for partial sentences, this doesn't seem to be one of them. Omitting the subject seems to be a popular shortcut, but it makes the documentation harder to read. It would be better to write: //TODO: This can be an expensive query. We should investigate cache // expiration, and perhaps only execute it every x minutes. This reminds me of two other points: First, think twice before omitting a word just to avoid going beyond column 80. Going to a second line isn't the end of the world. The second gets back to the issue of quotes or apostrophes. Often, when you feel a need to put a code fragment or other phrase into quotes, it's even better to just make it an indented paragraph. However, I wouldn't be surprised if Doxygen can't handle this properly, so I'm not sure there should be a rule around this. Gary
On Mon, 2007-01-08 at 10:43 -0500, Angela Byron wrote:
Excellent! Thanks for the summary, Derek!!
<snip />
Now, just one (technical) question.
Does this:
// @todo These queries are very expensive, see http://drupal.org/ node/105639.
...actually work and get parsed by doxygen?? I thought doxygen only worked on function headers. This would make @todo less useful, imo, since:
// TODO: this can be an expensive query. Perhaps only execute it every x minutes. Requires investigation into cache expiration. if ($user->uid) { db_query("UPDATE {users} SET access = %d WHERE uid = %d", time (), $user->uid); }
Tells me that the queries immediately following that line are expensive, vs.
/** * Reads a session variable. * * @todo the update query used here can be expensive.... */ function sess_read($key) {
Anyone know?
for phpdoc // @todo wouldn't work... /** @todo would word */
Also, as Robert brought up, // TODO, etc. have support in IDEs (Zend, etc.). Can someone check if @todo does as well?
-Angie
for phpdoc // @todo wouldn't work... /** @todo would word */
Most IDEs recognize // TODO: Whatevs. as valid todos. So for inline todos, those seem the way to go. For Doxygen, you're right that we should definitely use @todo. Are you suggesting that we use /* @todo */ for inline todos as well and Doxygen will pick up on those? Rob Roy Barreca Founder and COO Electronic Insight Corporation http://www.electronicinsight.com rob@electronicinsight.com Darrel O'Pry wrote:
On Mon, 2007-01-08 at 10:43 -0500, Angela Byron wrote:
Excellent! Thanks for the summary, Derek!!
<snip />
Now, just one (technical) question.
Does this:
// @todo These queries are very expensive, see http://drupal.org/ node/105639.
...actually work and get parsed by doxygen?? I thought doxygen only worked on function headers. This would make @todo less useful, imo, since:
// TODO: this can be an expensive query. Perhaps only execute it every x minutes. Requires investigation into cache expiration. if ($user->uid) { db_query("UPDATE {users} SET access = %d WHERE uid = %d", time (), $user->uid); }
Tells me that the queries immediately following that line are expensive, vs.
/** * Reads a session variable. * * @todo the update query used here can be expensive.... */ function sess_read($key) {
Anyone know?
for phpdoc // @todo wouldn't work... /** @todo would word */
Also, as Robert brought up, // TODO, etc. have support in IDEs (Zend, etc.). Can someone check if @todo does as well?
-Angie
I don't expect these are used by many, but adding what I use, FWIW: vim (command line for *NIX systems mainly) handles // TODO and // FIXME, and they are highlighted. It does not highlight doxygen (maybe needs a plugin, but not by default). Quanta (GUI IDE for KDE) does NOT highlight // TODO nor // FIXME, but does detect doxygen by default and highlights @todo and other doxygen keywords. @todo is only highlighted if it is in a doxygon block.
On 8-Jan-07, at 1:31 PM, Darrel O'Pry wrote:
for phpdoc // @todo wouldn't work... /** @todo would word */
OK. In that case, I'm going to need to -1 @todo. /** stuff **/ as inline comments sucks, because then you can't comment out large blocks of code easily, which is something essential to debugging. That coupled with everyone else saying that // TODO is supported in their editor of choice makes this a closed case, imo. -Angie
participants (6)
-
Angela Byron -
Darrel O'Pry -
Gary Feldman -
Greg Knaddison - GVS -
Khalid B -
Rob Barreca