We could use a cleanup of our code comments and documentation, mainly so that new code that's written has a consistent pattern to follow. In many cases it's not clear what we do, because we do differently each place. The attached patch is a first crack at only three files. Consistency issues and changes include:

* Comments are full sentences ending in periods.
* There is space between %param and %return sections in function documentation (do we want this? sometimes we do it, sometimes not).
* Function documentation begins with a single sentence in the form "Do such and such." rather than "Does such and such.", and has a space before any further comments.
* Where we have a Note: it is not all caps.
* Spelling, grammar, punctuation corrections.

If this is seen as worthwhile, I'd be willing to plug away and complete this patch.--though I'd welcome any other volunteers to take parts! I'd appreciate any feedback on the consistency issues--is what I've done in this prelimiary patch wanted?

Comments

webchick’s picture

+1 to this. I also volunteer to help, although my first priority for "free-time" hacking will be going to the installer. Judging by other "clean-up" type of patches, we might have better luck getting this in as several smaller patches rather than one large one. Would make it easier to pick away at it in between things, too. Maybe we could tag them with something like [CodeCleanUp] blahblah.file so that we can easily tell what's been done and what hasn't.

Also, one thing I noticed at the beginning:

+// These values should match the'role' table.

should be:

+// These values should match the 'role' table.

(missing space)

dries’s picture

Good idea. I'd start by creating some guidelines in the handbook, and getting a some level of agreement on it. Then we have some rules to adhere to, and committing these patches becomes a no-brainer.

dries’s picture

Status: Needs review » Needs work

I'm going to mark this 'code needs work'. Let's create the guidelines before we start changing everything.

nedjo’s picture

I've made some minor additions to the Doxygen formatting conventions handbook page, http://drupal.org/node/1354, about capitalization and blank lines. Most of the rest of what we need is already covered. I tried to post a comment on the Coding standards page, http://drupal.org/node/318, but got a validation error (and don't have edit access to the page), so here's the proposed additions to the "Comments" section:

Non-documentation comments should use capitalized sentences with punctuation and should be on a separate line immediately before the code line or block they reference. For example:


// Unselect all other contact categories.
db_query('UPDATE {contact} SET selected = 0');

If each line of a list needs a separate comment, the comments may be given on the same line and may be formatted to a uniform indent for readability.

All caps are used in comments only when referencing constants, e.g., TRUE.

So the changes and corrections would be:

* 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.

LaurenH’s picture

Speaking of spelling/grammar/punctuation issues, it seems that the habit of not capitalizing menu items or section headings in Drupal seems to be hard-coded into the system. This seems to me to really be more of a design preference rather than a proper stylistic rule, and it would make more sense to have them properly capitalized and then allow users to de-capitalize them for design purposes using CSS, if they so desire.

nedjo’s picture

StatusFileSize
new187.92 KB

Here's a partial patch. I've gone through the .inc files but haven't yet made more than a first pass for a couple of things through the.module files. I'm not going to be able to work on this for the next week and a half, so if anyone has an idle moment and wants to push this further, please do!

LAsan’s picture

Version: x.y.z » 7.x-dev

nedjo:Is this implemented in current versions?

Moving to cvs.

Gurpartap Singh’s picture

Component: base system » other
jody lynn’s picture

Status: Needs work » Closed (won't fix)

I'm closing this one because the patch is from 2006 and a great amount of work has happened in many many issues toward these improvements